Detects accidental multiple invocations of AnyInvocable<R(...)&&>::op…

…erator()&& by producing an error in debug mode, and clarifies that the behavior is undefined in the general case. PiperOrigin-RevId: 480392976 Change-Id: I2d4c6f213fa7c8747f125c9735272a8e47b9214b
abseil · Oct 11, 2022 · 73789eb · 73789eb
1 parent 845610e
commit 73789eb
Show file tree

Hide file tree

Showing 3 changed files with 64 additions and 12 deletions.
diff --git a/absl/functional/any_invocable.h b/absl/functional/any_invocable.h
@@ -148,6 +148,9 @@ ABSL_NAMESPACE_BEGIN
 //     // rvalue-reference qualified.
 //     std::move(continuation)(result_of_foo);
 //   }
+//
+// Attempting to call `absl::AnyInvocable` multiple times in such a case
+// results in undefined behavior.
 template <class Sig>
 class AnyInvocable : private internal_any_invocable::Impl<Sig> {
  private:

diff --git a/absl/functional/any_invocable_test.cc b/absl/functional/any_invocable_test.cc
@@ -16,6 +16,7 @@
 
 #include <cstddef>
 #include <initializer_list>
+#include <memory>
 #include <numeric>
 #include <type_traits>
 
@@ -1156,9 +1157,6 @@ TYPED_TEST_P(AnyInvTestMovable, ConversionConstructionUserDefinedType) {
 
   EXPECT_TRUE(static_cast<bool>(fun));
   EXPECT_EQ(29, TypeParam::ToThisParam(fun)(7, 8, 9).value);
-
-  EXPECT_TRUE(static_cast<bool>(fun));
-  EXPECT_EQ(38, TypeParam::ToThisParam(fun)(10, 11, 12).value);
 }
 
 TYPED_TEST_P(AnyInvTestMovable, ConversionConstructionVoidCovariance) {
@@ -1179,9 +1177,6 @@ TYPED_TEST_P(AnyInvTestMovable, ConversionAssignUserDefinedTypeEmptyLhs) {
 
   EXPECT_TRUE(static_cast<bool>(fun));
   EXPECT_EQ(29, TypeParam::ToThisParam(fun)(7, 8, 9).value);
-
-  EXPECT_TRUE(static_cast<bool>(fun));
-  EXPECT_EQ(38, TypeParam::ToThisParam(fun)(10, 11, 12).value);
 }
 
 TYPED_TEST_P(AnyInvTestMovable, ConversionAssignUserDefinedTypeNonemptyLhs) {
@@ -1193,9 +1188,6 @@ TYPED_TEST_P(AnyInvTestMovable, ConversionAssignUserDefinedTypeNonemptyLhs) {
 
   EXPECT_TRUE(static_cast<bool>(fun));
   EXPECT_EQ(29, TypeParam::ToThisParam(fun)(7, 8, 9).value);
-
-  EXPECT_TRUE(static_cast<bool>(fun));
-  EXPECT_EQ(38, TypeParam::ToThisParam(fun)(10, 11, 12).value);
 }
 
 TYPED_TEST_P(AnyInvTestMovable, ConversionAssignVoidCovariance) {
@@ -1414,6 +1406,41 @@ TYPED_TEST_P(AnyInvTestRvalue, ConversionAssignReferenceWrapper) {
       std::is_assignable<AnyInvType&, std::reference_wrapper<AddType>>::value));
 }
 
+TYPED_TEST_P(AnyInvTestRvalue, NonConstCrashesOnSecondCall) {
+  using AnyInvType = typename TypeParam::AnyInvType;
+  using AddType = typename TypeParam::AddType;
+
+  AnyInvType fun(absl::in_place_type<AddType>, 5);
+
+  EXPECT_TRUE(static_cast<bool>(fun));
+  std::move(fun)(7, 8, 9);
+
+  // Ensure we're still valid
+  EXPECT_TRUE(static_cast<bool>(fun));  // NOLINT(bugprone-use-after-move)
+
+#if !defined(NDEBUG) || ABSL_OPTION_HARDENED == 1
+  EXPECT_DEATH_IF_SUPPORTED(std::move(fun)(7, 8, 9), "");
+#endif
+}
+
+// Ensure that any qualifiers (in particular &&-qualifiers) do not affect
+// when the destructor is actually run.
+TYPED_TEST_P(AnyInvTestRvalue, QualifierIndependentObjectLifetime) {
+  using AnyInvType = typename TypeParam::AnyInvType;
+
+  auto refs = std::make_shared<std::nullptr_t>();
+  {
+    AnyInvType fun([refs](auto&&...) noexcept { return 0; });
+    EXPECT_FALSE(refs.unique());
+
+    std::move(fun)(7, 8, 9);
+
+    // Ensure destructor hasn't run even if rref-qualified
+    EXPECT_FALSE(refs.unique());
+  }
+  EXPECT_TRUE(refs.unique());
+}
+
 // NOTE: This test suite originally attempted to enumerate all possible
 // combinations of type properties but the build-time started getting too large.
 // Instead, it is now assumed that certain parameters are orthogonal and so
@@ -1670,7 +1697,9 @@ INSTANTIATE_TYPED_TEST_SUITE_P(NonRvalueCallNothrow, AnyInvTestNonRvalue,
 REGISTER_TYPED_TEST_SUITE_P(AnyInvTestRvalue,
                             ConversionConstructionReferenceWrapper,
                             NonMoveableResultType,
-                            ConversionAssignReferenceWrapper);
+                            ConversionAssignReferenceWrapper,
+                            NonConstCrashesOnSecondCall,
+                            QualifierIndependentObjectLifetime);
 
 INSTANTIATE_TYPED_TEST_SUITE_P(RvalueCallMayThrow, AnyInvTestRvalue,
                                TestParameterListRvalueQualifiersCallMayThrow);

diff --git a/absl/functional/internal/any_invocable.h b/absl/functional/internal/any_invocable.h
@@ -809,11 +809,31 @@ using CanAssignReferenceWrapper = TrueAlias<
         : Core(absl::in_place_type<absl::decay_t<T> inv_quals>,                \
                std::forward<Args>(args)...) {}                                 \
                                                                                \
+    InvokerType<noex, ReturnType, P...>* ExtractInvoker() cv {                 \
+      using QualifiedTestType = int cv ref;                                    \
+      auto* invoker = this->invoker_;                                          \
+      if (!std::is_const<QualifiedTestType>::value &&                          \
+          std::is_rvalue_reference<QualifiedTestType>::value) {                \
+        ABSL_HARDENING_ASSERT([this]() {                                       \
+          /* We checked that this isn't const above, so const_cast is safe */  \
+          const_cast<Impl*>(this)->invoker_ =                                  \
+              [](TypeErasedState*,                                             \
+                 ForwardedParameterType<P>...) noexcept(noex) -> ReturnType {  \
+            ABSL_HARDENING_ASSERT(false && "AnyInvocable use-after-move");     \
+            std::terminate();                                                  \
+          };                                                                   \
+          return this->HasValue();                                             \
+        }());                                                                  \
+      }                                                                        \
+      return invoker;                                                          \
+    }                                                                          \
+                                                                               \
     /*The actual invocation operation with the proper signature*/              \
     ReturnType operator()(P... args) cv ref noexcept(noex) {                   \
       assert(this->invoker_ != nullptr);                                       \
-      return this->invoker_(const_cast<TypeErasedState*>(&this->state_),       \
-                            static_cast<ForwardedParameterType<P>>(args)...);  \
+      return this->ExtractInvoker()(                                           \
+          const_cast<TypeErasedState*>(&this->state_),                         \
+          static_cast<ForwardedParameterType<P>>(args)...);                    \
     }                                                                          \
   }