(kvrocks) branch unstable updated: fix(common): prevent ScopeExit cleanup after move (#3696)

[hulk]

This is an automated email from the ASF dual-hosted git repository.

git-hulk pushed a commit to branch unstable
in repository https://gitbox.apache.org/repos/asf/kvrocks.git


The following commit(s) were added to refs/heads/unstable by this push:
     new a42c431b0 fix(common): prevent ScopeExit cleanup after move (#3696)
a42c431b0 is described below

commit a42c431b093757c90b46fb88f6aeb97c170fb342
Author: nkysg <nkysg...@gmail.com>
AuthorDate: Wed Oct 21 21:15:19 2026 +0800

    fix(common): prevent ScopeExit cleanup after move (#3696)
    
    Summary
    
    Prevent `ScopeExit` from running its cleanup callback twice after move
    construction.
    
    - Disable the moved-from guard after its callback has been successfully
    moved.
    - Preserve the moved-to guard's enabled state.
    - Keep the source guard enabled if moving the callback throws.
    
    Without this change, both the moved-to and moved-from guards could
    invoke a non-idempotent cleanup callback during destruction.
    
    ## Tests
    
    - Added focused unit coverage for move construction and single cleanup
    execution.
    - `./x.py format`
    - LLVM 18 `clang-tidy` on `scope_exit_test.cc`.
    - Ran `ScopeExit.MoveConstructionRunsCleanupOnce` successfully.
    
    ## AI-assisted contribution
    
    OpenAI Codex was used to draft the unit tests for this change.
---
 src/common/scope_exit.h                            |  2 +-
 .../cppunit/scope_exit_test.cc                     | 41 +++++++---------------
 2 files changed, 14 insertions(+), 29 deletions(-)

diff --git a/src/common/scope_exit.h b/src/common/scope_exit.h
index 84b9e14ce..2faeddfb1 100644
--- a/src/common/scope_exit.h
+++ b/src/common/scope_exit.h
@@ -29,7 +29,7 @@ struct ScopeExit {
   explicit ScopeExit(F f) : enabled(true), f(std::move(f)) {}
 
   ScopeExit(const ScopeExit&) = delete;
-  ScopeExit(ScopeExit&& se) : enabled(se.enabled), f(std::move(se.f)) {}
+  ScopeExit(ScopeExit&& se) : enabled(se.enabled), f(std::move(se.f)) { 
se.Disable(); }
 
   ScopeExit& operator=(const ScopeExit&) = delete;
 
diff --git a/src/common/scope_exit.h b/tests/cppunit/scope_exit_test.cc
similarity index 53%
copy from src/common/scope_exit.h
copy to tests/cppunit/scope_exit_test.cc
index 84b9e14ce..6f95fb787 100644
--- a/src/common/scope_exit.h
+++ b/tests/cppunit/scope_exit_test.cc
@@ -15,38 +15,23 @@
  * KIND, either express or implied.  See the License for the
  * specific language governing permissions and limitations
  * under the License.
- *
  */
 
-#pragma once
-
-#include <utility>
+#include "common/scope_exit.h"
 
-// ref to https://en.cppreference.com/w/cpp/experimental/scope_exit
-template <typename F>
-struct ScopeExit {
-  explicit ScopeExit(F f, bool enabled) : enabled(enabled), f(std::move(f)) {}
-  explicit ScopeExit(F f) : enabled(true), f(std::move(f)) {}
+#include <gtest/gtest.h>
 
-  ScopeExit(const ScopeExit&) = delete;
-  ScopeExit(ScopeExit&& se) : enabled(se.enabled), f(std::move(se.f)) {}
-
-  ScopeExit& operator=(const ScopeExit&) = delete;
+#include <utility>
 
-  ~ScopeExit() {
-    if (enabled) f();
+TEST(ScopeExit, MoveConstructionRunsCleanupOnce) {
+  int cleanup_count = 0;
+  {
+    auto source = MakeScopeExit([&cleanup_count] { ++cleanup_count; });
+    {
+      auto target = std::move(source);
+      EXPECT_TRUE(target.enabled);
+    }
+    EXPECT_EQ(cleanup_count, 1);
   }
-
-  void Enable() { enabled = true; }
-
-  void Disable() { enabled = false; }
-
-  bool enabled;
-  F f;
-};
-
-// use CTAD in C++17 or above
-template <typename F>
-ScopeExit<F> MakeScopeExit(F&& f, bool enabled = true) {
-  return ScopeExit<F>(std::forward<F>(f), enabled);
+  EXPECT_EQ(cleanup_count, 1);
 }