5 files changed, 79 insertions, 16 deletions

diff --git a/absl/cleanup/BUILD.bazel b/absl/cleanup/BUILD.bazel
index 5cca898f..2154d9f1 100644
--- a/absl/cleanup/BUILD.bazel
+++ b/absl/cleanup/BUILD.bazel
@@ -12,7 +12,6 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-load("@rules_cc//cc:defs.bzl", "cc_library", "cc_test")
 load(
     "//absl:copts/configure_copts.bzl",
     "ABSL_DEFAULT_COPTS",
diff --git a/absl/cleanup/CMakeLists.txt b/absl/cleanup/CMakeLists.txt
index a2dd78a8..f5af40b4 100644
--- a/absl/cleanup/CMakeLists.txt
+++ b/absl/cleanup/CMakeLists.txt
@@ -12,6 +12,7 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
+# Internal-only target, do not depend on directly.
 absl_cc_library(
   NAME
     cleanup_internal
@@ -51,5 +52,5 @@ absl_cc_test(
     absl::cleanup
     absl::config
     absl::utility
-    gmock_main
+    GTest::gmock_main
 )
diff --git a/absl/cleanup/cleanup.h b/absl/cleanup/cleanup.h
index 61b53d55..960ccd08 100644
--- a/absl/cleanup/cleanup.h
+++ b/absl/cleanup/cleanup.h
@@ -86,25 +86,25 @@ class ABSL_MUST_USE_RESULT Cleanup final {
                 "Callbacks that return values are not supported.");
 
  public:
-  Cleanup(Callback callback)  // NOLINT
-      : storage_(std::move(callback), /* is_callback_engaged = */ true) {}
+  Cleanup(Callback callback) : storage_(std::move(callback)) {}  // NOLINT
 
   Cleanup(Cleanup&& other) = default;
 
   void Cancel() && {
     ABSL_HARDENING_ASSERT(storage_.IsCallbackEngaged());
-    storage_.DisengageCallback();
+    storage_.DestroyCallback();
   }
 
   void Invoke() && {
     ABSL_HARDENING_ASSERT(storage_.IsCallbackEngaged());
-    storage_.DisengageCallback();
     storage_.InvokeCallback();
+    storage_.DestroyCallback();
   }
 
   ~Cleanup() {
     if (storage_.IsCallbackEngaged()) {
       storage_.InvokeCallback();
+      storage_.DestroyCallback();
     }
   }
 
diff --git a/absl/cleanup/cleanup_test.cc b/absl/cleanup/cleanup_test.cc
index 792595d6..46b88589 100644
--- a/absl/cleanup/cleanup_test.cc
+++ b/absl/cleanup/cleanup_test.cc
@@ -264,4 +264,48 @@ TYPED_TEST(CleanupTest, Move) {
   EXPECT_FALSE(called);  // Destructor shouldn't invoke the callback
 }
 
+int DestructionCount = 0;
+
+struct DestructionCounter {
+  void operator()() {}
+
+  ~DestructionCounter() { ++DestructionCount; }
+};
+
+TYPED_TEST(CleanupTest, DestructorDestroys) {
+  {
+    auto cleanup =
+        absl::MakeCleanup(TypeParam::AsCallback(DestructionCounter()));
+    DestructionCount = 0;
+  }
+
+  EXPECT_EQ(DestructionCount, 1);  // Engaged cleanup destroys
+}
+
+TYPED_TEST(CleanupTest, CancelDestroys) {
+  {
+    auto cleanup =
+        absl::MakeCleanup(TypeParam::AsCallback(DestructionCounter()));
+    DestructionCount = 0;
+
+    std::move(cleanup).Cancel();
+    EXPECT_EQ(DestructionCount, 1);  // Cancel destroys
+  }
+
+  EXPECT_EQ(DestructionCount, 1);  // Canceled cleanup does not double destroy
+}
+
+TYPED_TEST(CleanupTest, InvokeDestroys) {
+  {
+    auto cleanup =
+        absl::MakeCleanup(TypeParam::AsCallback(DestructionCounter()));
+    DestructionCount = 0;
+
+    std::move(cleanup).Invoke();
+    EXPECT_EQ(DestructionCount, 1);  // Invoke destroys
+  }
+
+  EXPECT_EQ(DestructionCount, 1);  // Invoked cleanup does not double destroy
+}
+
 }  // namespace
diff --git a/absl/cleanup/internal/cleanup.h b/absl/cleanup/internal/cleanup.h
index b4c40737..2783fcb7 100644
--- a/absl/cleanup/internal/cleanup.h
+++ b/absl/cleanup/internal/cleanup.h
@@ -15,10 +15,12 @@
 #ifndef ABSL_CLEANUP_INTERNAL_CLEANUP_H_
 #define ABSL_CLEANUP_INTERNAL_CLEANUP_H_
 
+#include <new>
 #include <type_traits>
 #include <utility>
 
 #include "absl/base/internal/invoke.h"
+#include "absl/base/macros.h"
 #include "absl/base/thread_annotations.h"
 #include "absl/utility/utility.h"
 
@@ -45,14 +47,22 @@ class Storage {
  public:
   Storage() = delete;
 
-  Storage(Callback callback, bool is_callback_engaged)
-      : callback_(std::move(callback)),
-        is_callback_engaged_(is_callback_engaged) {}
+  explicit Storage(Callback callback) {
+    // Placement-new into a character buffer is used for eager destruction when
+    // the cleanup is invoked or cancelled. To ensure this optimizes well, the
+    // behavior is implemented locally instead of using an absl::optional.
+    ::new (GetCallbackBuffer()) Callback(std::move(callback));
+    is_callback_engaged_ = true;
+  }
+
+  Storage(Storage&& other) {
+    ABSL_HARDENING_ASSERT(other.IsCallbackEngaged());
 
-  Storage(Storage&& other)
-      : callback_(std::move(other.callback_)),
-        is_callback_engaged_(
-            absl::exchange(other.is_callback_engaged_, false)) {}
+    ::new (GetCallbackBuffer()) Callback(std::move(other.GetCallback()));
+    is_callback_engaged_ = true;
+
+    other.DestroyCallback();
+  }
 
   Storage(const Storage& other) = delete;
 
@@ -60,17 +70,26 @@ class Storage {
 
   Storage& operator=(const Storage& other) = delete;
 
+  void* GetCallbackBuffer() { return static_cast<void*>(+callback_buffer_); }
+
+  Callback& GetCallback() {
+    return *reinterpret_cast<Callback*>(GetCallbackBuffer());
+  }
+
   bool IsCallbackEngaged() const { return is_callback_engaged_; }
 
-  void DisengageCallback() { is_callback_engaged_ = false; }
+  void DestroyCallback() {
+    is_callback_engaged_ = false;
+    GetCallback().~Callback();
+  }
 
   void InvokeCallback() ABSL_NO_THREAD_SAFETY_ANALYSIS {
-    std::move(callback_)();
+    std::move(GetCallback())();
   }
 
  private:
-  Callback callback_;
   bool is_callback_engaged_;
+  alignas(Callback) char callback_buffer_[sizeof(Callback)];
 };
 
 }  // namespace cleanup_internal