[PATCH] D34331: func.wrap.func.con: Unset function before destroying anything

[Duncan P. N. Exon Smith via Phabricator via cfe-commits]

dexonsmith created this revision.

Be defensive against a reentrant std::function::operator=(), in case the held 
function object has a non-trivial destructor.  Destroying the function object 
in-place can lead to the destructor being called twice.

rdar://problem/32836603


https://reviews.llvm.org/D34331

Files:
  libcxx/include/functional
  
libcxx/test/libcxx/utilities/function.objects/func.wrap/func.wrap.func/func.wrap.func.con/move_reentrant.pass.cpp
  
libcxx/test/libcxx/utilities/function.objects/func.wrap/func.wrap.func/func.wrap.func.con/nullptr_t_assign_reentrant.pass.cpp


Index: libcxx/test/libcxx/utilities/function.objects/func.wrap/func.wrap.func/func.wrap.func.con/nullptr_t_assign_reentrant.pass.cpp
===================================================================
--- /dev/null
+++ libcxx/test/libcxx/utilities/function.objects/func.wrap/func.wrap.func/func.wrap.func.con/nullptr_t_assign_reentrant.pass.cpp
@@ -0,0 +1,44 @@
+//===----------------------------------------------------------------------===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+// <functional>
+
+// class function<R(ArgTypes...)>
+
+// function& operator=(nullptr_t);
+
+#include <functional>
+#include <cassert>
+
+struct A
+{
+  static std::function<void()> global;
+  static bool cancel;
+
+  ~A() {
+    asm("");
+    if (cancel)
+      global = nullptr;
+  }
+  void operator()() {}
+};
+
+std::function<void()> A::global;
+bool A::cancel = false;
+
+int main()
+{
+  A::global = A();
+  assert(A::global.target<A>());
+
+  // Check that we don't recurse in A::~A().
+  A::cancel = true;
+  A::global = nullptr;
+  assert(!A::global.target<A>());
+}
Index: libcxx/test/libcxx/utilities/function.objects/func.wrap/func.wrap.func/func.wrap.func.con/move_reentrant.pass.cpp
===================================================================
--- /dev/null
+++ libcxx/test/libcxx/utilities/function.objects/func.wrap/func.wrap.func/func.wrap.func.con/move_reentrant.pass.cpp
@@ -0,0 +1,44 @@
+//===----------------------------------------------------------------------===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+// <functional>
+
+// class function<R(ArgTypes...)>
+
+// function& operator=(function &&);
+
+#include <functional>
+#include <cassert>
+
+struct A
+{
+  static std::function<void()> global;
+  static bool cancel;
+
+  ~A() {
+    asm("");
+    if (cancel)
+      global = std::function<void()>(nullptr);
+  }
+  void operator()() {}
+};
+
+std::function<void()> A::global;
+bool A::cancel = false;
+
+int main()
+{
+  A::global = A();
+  assert(A::global.target<A>());
+
+  // Check that we don't recurse in A::~A().
+  A::cancel = true;
+  A::global = std::function<void()>(nullptr);
+  assert(!A::global.target<A>());
+}
Index: libcxx/include/functional
===================================================================
--- libcxx/include/functional
+++ libcxx/include/functional
@@ -1821,11 +1821,7 @@
 function<_Rp(_ArgTypes...)>&
 function<_Rp(_ArgTypes...)>::operator=(function&& __f) _NOEXCEPT
 {
-    if ((void *)__f_ == &__buf_)
-        __f_->destroy();
-    else if (__f_)
-        __f_->destroy_deallocate();
-    __f_ = 0;
+    function::operator=(nullptr);
     if (__f.__f_ == 0)
         __f_ = 0;
     else if ((void *)__f.__f_ == &__f.__buf_)
@@ -1845,11 +1841,12 @@
 function<_Rp(_ArgTypes...)>&
 function<_Rp(_ArgTypes...)>::operator=(nullptr_t) _NOEXCEPT
 {
-    if ((void *)__f_ == &__buf_)
-        __f_->destroy();
-    else if (__f_)
-        __f_->destroy_deallocate();
+    __base* __t = __f_;
     __f_ = 0;
+    if ((void *)__t == &__buf_)
+        __t->destroy();
+    else if (__t)
+        __t->destroy_deallocate();
     return *this;
 }
 

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits