From df2804dc1d4d9c983db407e145d69dcb1b065d3b Mon Sep 17 00:00:00 2001
From: Yingxin Cheng <yingxin.cheng@intel.com>
Date: Tue, 7 May 2024 13:45:40 +0800
Subject: [PATCH] crimson/common/operation: cleanup, merge exit() into
 ~PipelineExitBarrierI()

Signed-off-by: Yingxin Cheng <yingxin.cheng@intel.com>
---
 src/crimson/common/operation.h | 40 ++++++++++------------------------
 1 file changed, 12 insertions(+), 28 deletions(-)

diff --git a/src/crimson/common/operation.h b/src/crimson/common/operation.h
index 1cdc672805f..284370fda14 100644
--- a/src/crimson/common/operation.h
+++ b/src/crimson/common/operation.h
@@ -478,11 +478,6 @@ public:
   virtual std::optional<seastar::future<>> wait() = 0;
 
   /// Releases pipeline resources, after or without waiting
-  // FIXME: currently, exit() will discard the associated future even if it is
-  // still unresolved, which is discouraged by seastar.
-  virtual void exit() = 0;
-
-  /// Must ensure that resources are released, likely by calling exit()
   virtual ~PipelineExitBarrierI() {}
 };
 
@@ -627,16 +622,10 @@ class OrderedExclusivePhaseT : public PipelineStageIT<T> {
       return std::nullopt;
     }
 
-    void exit() final {
-      if (phase) {
-        assert(phase->core == seastar::this_shard_id());
-        phase->exit(op_id);
-        phase = nullptr;
-      }
-    }
-
     ~ExitBarrier() final {
-      exit();
+      assert(phase);
+      assert(phase->core == seastar::this_shard_id());
+      phase->exit(op_id);
     }
   };
 
@@ -727,26 +716,23 @@ private:
       return trigger.maybe_record_exit_barrier(std::move(ret));
     }
 
-    void exit() final {
+    ~ExitBarrier() final {
+      assert(phase);
+      assert(phase->core == seastar::this_shard_id());
       if (barrier) {
-        assert(phase);
-        assert(phase->core == seastar::this_shard_id());
+        // wait() hasn't been called
+
+        // FIXME: should not discard future,
+        // it's discouraged by seastar and may cause shutdown issues.
         std::ignore = std::move(*barrier
         ).then([phase=this->phase] {
           phase->mutex.unlock();
         });
-        barrier = std::nullopt;
-        phase = nullptr;
-      } else if (phase) {
-        assert(phase->core == seastar::this_shard_id());
+      } else {
+        // wait() has been called
         phase->mutex.unlock();
-        phase = nullptr;
       }
     }
-
-    ~ExitBarrier() final {
-      exit();
-    }
   };
 
 public:
@@ -778,8 +764,6 @@ class UnorderedStageT : public PipelineStageIT<T> {
       return std::nullopt;
     }
 
-    void exit() final {}
-
     ~ExitBarrier() final {}
   };
 
-- 
2.39.5