From 03439afb970453a0ae932cd0fc8f3ad24f40d8e5 Mon Sep 17 00:00:00 2001
From: Dave Watson <davejwatson@fb.com>
Date: Wed, 27 May 2015 11:20:40 -0700
Subject: [PATCH] Fix cancel in ThreadWheelTimeKeeper

Summary: This is actually a bug, future.cancel() doesn't work with the current THreadWheelTimekeeper, because cancel() only works from the eventBase thread.

Test Plan: added unittest.  Crashes before, passes now

Reviewed By: hans@fb.com

Subscribers: doug, folly-diffs@, jsedgwick, yfeldblum, chalfant

FB internal diff: D2091531

Signature: t1:2091531:1432224024:4aa5dd71de15b1344034a414d47c97ffaba68949
---
 folly/futures/detail/ThreadWheelTimekeeper.cpp | 16 ++++++++++------
 folly/futures/test/TimekeeperTest.cpp          |  6 ++++++
 2 files changed, 16 insertions(+), 6 deletions(-)

diff --git a/folly/futures/detail/ThreadWheelTimekeeper.cpp b/folly/futures/detail/ThreadWheelTimekeeper.cpp
index f861af23..294b71fa 100644
--- a/folly/futures/detail/ThreadWheelTimekeeper.cpp
+++ b/folly/futures/detail/ThreadWheelTimekeeper.cpp
@@ -27,9 +27,9 @@ namespace {
   // Our Callback object for HHWheelTimer
   struct WTCallback : public folly::HHWheelTimer::Callback {
     // Only allow creation by this factory, to ensure heap allocation.
-    static WTCallback* create() {
+    static WTCallback* create(EventBase* base) {
       // optimization opportunity: memory pool
-      return new WTCallback();
+      return new WTCallback(base);
     }
 
     Future<void> getFuture() {
@@ -37,9 +37,11 @@ namespace {
     }
 
    protected:
+    EventBase* base_;
     Promise<void> promise_;
 
-    explicit WTCallback() {
+    explicit WTCallback(EventBase* base)
+        : base_(base) {
       promise_.setInterruptHandler(
         std::bind(&WTCallback::interruptHandler, this));
     }
@@ -50,8 +52,10 @@ namespace {
     }
 
     void interruptHandler() {
-      cancelTimeout();
-      delete this;
+      base_->runInEventBaseThread([=] {
+        cancelTimeout();
+        delete this;
+      });
     }
   };
 
@@ -78,7 +82,7 @@ ThreadWheelTimekeeper::~ThreadWheelTimekeeper() {
 }
 
 Future<void> ThreadWheelTimekeeper::after(Duration dur) {
-  auto cob = WTCallback::create();
+  auto cob = WTCallback::create(&eventBase_);
   auto f = cob->getFuture();
   eventBase_.runInEventBaseThread([=]{
     wheelTimer_->scheduleTimeout(cob, dur);
diff --git a/folly/futures/test/TimekeeperTest.cpp b/folly/futures/test/TimekeeperTest.cpp
index ba792bb3..0fdac19a 100644
--- a/folly/futures/test/TimekeeperTest.cpp
+++ b/folly/futures/test/TimekeeperTest.cpp
@@ -25,6 +25,7 @@ using Duration = folly::Duration;
 
 std::chrono::milliseconds const one_ms(1);
 std::chrono::milliseconds const awhile(10);
+std::chrono::seconds const too_long(10);
 
 std::chrono::steady_clock::time_point now() {
   return std::chrono::steady_clock::now();
@@ -154,6 +155,11 @@ TEST(Timekeeper, onTimeoutVoid) {
   // just testing compilation here
 }
 
+TEST(Timekeeper, interruptDoesntCrash) {
+  auto f = futures::sleep(too_long);
+  f.cancel();
+}
+
 // TODO(5921764)
 /*
 TEST(Timekeeper, onTimeoutPropagates) {
-- 
2.34.1