From: Radoslaw Zarzynski <rzarzyns@redhat.com>
Date: Tue, 26 Jan 2021 21:54:20 +0000 (+0100)
Subject: crimson: fix dangling ClientRequest::this pointer.
X-Git-Tag: v17.1.0~3157^2
X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=refs%2Fpull%2F39093%2Fhead;p=ceph.git

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 2957639c63471..10f3395906ab7 100644
--- 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() {