From: Xuehan Xu <xxhdx1985126@gmail.com>
Date: Sun, 16 May 2021 15:23:19 +0000 (+0800)
Subject: crimson/osd: make do_osd_ops receive lvalue reference to osd ops vector
X-Git-Tag: v17.1.0~1944^2
X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=0a0848ddfd29228ac23e239f1a48e050e96b564f;p=ceph.git

crimson/osd: make do_osd_ops receive lvalue reference to osd ops vector

otherwise any async execution of lambdas in PG::do_osd_ops_execute() may
reference a outdated osd_op

Signed-off-by: Xuehan Xu <xxhdx1985126@gmail.com>
---

diff --git a/src/crimson/osd/osd_operations/internal_client_request.cc b/src/crimson/osd/osd_operations/internal_client_request.cc
index fcc64c545a40..9b44695a0bc0 100644
--- a/src/crimson/osd/osd_operations/internal_client_request.cc
+++ b/src/crimson/osd/osd_operations/internal_client_request.cc
@@ -58,39 +58,41 @@ seastar::future<> InternalClientRequest::start()
               handle.enter(pp().get_obc)
             ).then_interruptible([this] () -> PG::load_obc_iertr::future<> {
               logger().debug("{}: getting obc lock", *this);
-              auto osd_ops = create_osd_ops();
-              logger().debug("InternalClientRequest: got {} OSDOps to execute",
-                             std::size(osd_ops));
-              [[maybe_unused]] const int ret = op_info.set_from_op(
-                std::as_const(osd_ops), pg->get_pgid().pgid, *pg->get_osdmap());
-              assert(ret == 0);
-              return pg->with_locked_obc(get_target_oid(), op_info,
-                [osd_ops=std::move(osd_ops), this](auto obc) {
-                return with_blocking_future_interruptible<IOInterruptCondition>(
-                  handle.enter(pp().process)
-                ).then_interruptible(
-                  [obc=std::move(obc), osd_ops=std::move(osd_ops), this] {
-                  return pg->do_osd_ops(
-                    std::move(obc),
-                    std::move(osd_ops),
-                    std::as_const(op_info),
-                    get_do_osd_ops_params(),
-                    [] {
-                      return PG::do_osd_ops_iertr::now();
-                    },
-                    [] (const std::error_code& e) {
-                      return PG::do_osd_ops_iertr::now();
-                    }
-                  ).safe_then_unpack_interruptible(
-                    [](auto submitted, auto all_completed) {
-                      return all_completed.handle_error_interruptible(
-                        crimson::ct_error::eagain::handle([] {
-                          return seastar::now();
-                        }));
-                    }, crimson::ct_error::eagain::handle([] {
-                      return interruptor::now();
-                    })
-                  );
+              return seastar::do_with(create_osd_ops(),
+                [this](auto& osd_ops) mutable {
+                logger().debug("InternalClientRequest: got {} OSDOps to execute",
+                               std::size(osd_ops));
+                [[maybe_unused]] const int ret = op_info.set_from_op(
+                  std::as_const(osd_ops), pg->get_pgid().pgid, *pg->get_osdmap());
+                assert(ret == 0);
+                return pg->with_locked_obc(get_target_oid(), op_info,
+                  [&osd_ops, this](auto obc) {
+                  return with_blocking_future_interruptible<IOInterruptCondition>(
+                    handle.enter(pp().process)
+                  ).then_interruptible(
+                    [obc=std::move(obc), &osd_ops, this] {
+                    return pg->do_osd_ops(
+                      std::move(obc),
+                      osd_ops,
+                      std::as_const(op_info),
+                      get_do_osd_ops_params(),
+                      [] {
+                        return PG::do_osd_ops_iertr::now();
+                      },
+                      [] (const std::error_code& e) {
+                        return PG::do_osd_ops_iertr::now();
+                      }
+                    ).safe_then_unpack_interruptible(
+                      [](auto submitted, auto all_completed) {
+                        return all_completed.handle_error_interruptible(
+                          crimson::ct_error::eagain::handle([] {
+                            return seastar::now();
+                          }));
+                      }, crimson::ct_error::eagain::handle([] {
+                        return interruptor::now();
+                      })
+                    );
+                  });
                 });
               });
             }).handle_error_interruptible(PG::load_obc_ertr::all_same_way([] {
diff --git a/src/crimson/osd/pg.cc b/src/crimson/osd/pg.cc
index 3ff19636d782..04f33c53b9b2 100644
--- a/src/crimson/osd/pg.cc
+++ b/src/crimson/osd/pg.cc
@@ -705,7 +705,7 @@ template <class Ret, class SuccessFunc, class FailureFunc>
 PG::do_osd_ops_iertr::future<PG::pg_rep_op_fut_t<Ret>>
 PG::do_osd_ops_execute(
   OpsExecuter&& ox,
-  std::vector<OSDOp> ops,
+  std::vector<OSDOp>& ops,
   const OpInfo &op_info,
   SuccessFunc&& success_func,
   FailureFunc&& failure_func)
@@ -830,7 +830,7 @@ PG::do_osd_ops(
 PG::do_osd_ops_iertr::future<PG::pg_rep_op_fut_t<>>
 PG::do_osd_ops(
   ObjectContextRef obc,
-  std::vector<OSDOp> ops,
+  std::vector<OSDOp>& ops,
   const OpInfo &op_info,
   const do_osd_ops_params_t& msg_params,
   do_osd_ops_success_func_t success_func,
@@ -840,7 +840,7 @@ PG::do_osd_ops(
     std::move(obc), op_info, get_pool().info, get_backend(), msg_params);
   return do_osd_ops_execute<void>(
     std::move(*ox),
-    std::move(ops),
+    ops,
     std::as_const(op_info),
     std::move(success_func),
     std::move(failure_func)
diff --git a/src/crimson/osd/pg.h b/src/crimson/osd/pg.h
index 45655d25e336..d7f9afcaa8dc 100644
--- a/src/crimson/osd/pg.h
+++ b/src/crimson/osd/pg.h
@@ -588,7 +588,7 @@ private:
   struct do_osd_ops_params_t;
   do_osd_ops_iertr::future<pg_rep_op_fut_t<>> do_osd_ops(
     ObjectContextRef obc,
-    std::vector<OSDOp> ops,
+    std::vector<OSDOp>& ops,
     const OpInfo &op_info,
     const do_osd_ops_params_t& params,
     do_osd_ops_success_func_t success_func,
@@ -596,7 +596,7 @@ private:
   template <class Ret, class SuccessFunc, class FailureFunc>
   do_osd_ops_iertr::future<pg_rep_op_fut_t<Ret>> do_osd_ops_execute(
     OpsExecuter&& ox,
-    std::vector<OSDOp> ops,
+    std::vector<OSDOp>& ops,
     const OpInfo &op_info,
     SuccessFunc&& success_func,
     FailureFunc&& failure_func);