crimson: fix dangling ClientRequest::this pointer.

`ShardServices::start_operation<T>()` spawns an operation,
calls `start()` on it and returns a future plus a smart
pointer controlling its life-time. Callers are responsible
to ensure the pointer doesn't go out-of-scope before
the entire execution is finished. This is error-prone.

`OSD::handle_osd_op()` forgets about its responsibility
which results in dangling `this` pointer of `ClientRequest`.
I believe the problem is much wider spread and the class
is just the tip of the iceberg.

In this commit `start_operation<T>()` is altered to extend
the life-time a bit. However, this isn't an ultimate solution
as callers are still able to e.g. put extra `this`-catching
lambdas on the returned future. A new `with_operation<T>()`-
like interface is expected as a follow-up.

Signed-off-by: Radoslaw Zarzynski <rzarzyns@redhat.com>

diff --git a/src/crimson/osd/shard_services.h b/src/crimson/osd/shard_services.h
index 2957639c6347139c69aecdba27215e2db4848c1b..10f3395906ab704859e31ff9209f0df317f129cd 100644 (file)
--- a/src/crimson/osd/shard_services.h
+++ b/src/crimson/osd/shard_services.h
@@ -97,7 +97,13 @@ public:
       throw crimson::common::system_shutdown_exception();
     }
     auto op = registry.create_operation<T>(std::forward<Args>(args)...);
-    return std::make_pair(op, op->start());
+    auto fut = op->start().then([op /* by copy */] {
+      // ensure the op's lifetime is appropriate. It is not enough to
+      // guarantee it's alive at the scheduling stages (i.e. `then()`
+      // calling) but also during the actual execution (i.e. when passed
+      // lambdas are actually run).
+    });
+    return std::make_pair(std::move(op), std::move(fut));
   }
 
   seastar::future<> stop() {