race in Future destructor

Summary:
@mnd wrote some Wangle code that would trip up with a `bad_function_call` exception. This Shouldn't Happen™ but the exception comes from trying to call a `std::function` which is null. We pretty thoroughly examined his usage and didn't find any problems, and this patch seems to make the error go away. See #4207781 for more details.

And reasoning about it, it makes sense. Inline comments explain the race.

Test Plan: Alas, I haven't been able to get a minimal repro and therefore a regression unit test. It's a hard race to trigger. I still don't understand why Matt's code does it.

Reviewed By: davejwatson@fb.com

FB internal diff: D1304001

diff --git a/folly/wangle/Future-inl.h b/folly/wangle/Future-inl.h
index 0e19cc7993ddc7b926e5a00be8fec4e72b2b8f07..e563b86b657bf134f8bfbb4e5b84353b8c64a6ef 100644 (file)
--- a/folly/wangle/Future-inl.h
+++ b/folly/wangle/Future-inl.h
@@ -44,11 +44,7 @@ Future<T>& Future<T>::operator=(Future<T>&& other) {
 template <class T>
 Future<T>::~Future() {
   if (obj_) {
-    if (obj_->ready()) {
-      delete obj_;
-    } else {
-      setContinuation([](Try<T>&&) {}); // detach
-    }
+    setContinuation([](Try<T>&&) {}); // detach
   }
 }