Callbacks should ref the HHWheelTimer

Summary: Callbacks sometimes outlive the `HHWheelTimer` that they reference. Then the `Callback` tries to reference the dead `HHWheelTimer` and it could either misbehave or crash. This was caught reliably by ASAN tests.

Since `HHWheelTimer` already supports intrusive ref counting, the solution is to acquire a reference within the `Callback`.

Reviewed By: djwatson

Differential Revision: D2617966

fb-gh-sync-id: 02be9ffc5851c269d5933288a17ad394b33ac2dd

diff --git a/folly/io/async/HHWheelTimer.cpp b/folly/io/async/HHWheelTimer.cpp
index 1fd5e6d25d81b041bf691c154b610b63664bd456..c62fb93df58b4d41dd0e2fd6d5a859ebefc44c4a 100644 (file)
--- a/folly/io/async/HHWheelTimer.cpp
+++ b/folly/io/async/HHWheelTimer.cpp
@@ -21,6 +21,7 @@
 #include <folly/io/async/HHWheelTimer.h>
 #include <folly/io/async/Request.h>
 
+#include <folly/Optional.h>
 #include <folly/ScopeGuard.h>
 
 #include <cassert>
@@ -54,6 +55,7 @@ void HHWheelTimer::Callback::setScheduled(HHWheelTimer* wheel,
   assert(wheel_ == nullptr);
   assert(expiration_ == milliseconds(0));
 
+  wheelGuard_ = DestructorGuard(wheel);
   wheel_ = wheel;
 
   // Only update the now_ time if we're not in a timeout expired callback
@@ -72,6 +74,7 @@ void HHWheelTimer::Callback::cancelTimeoutImpl() {
   hook_.unlink();
 
   wheel_ = nullptr;
+  wheelGuard_ = folly::none;
   expiration_ = milliseconds(0);
 }
 

diff --git a/folly/io/async/HHWheelTimer.h b/folly/io/async/HHWheelTimer.h
index 0609ea666a71885b6637f93dd89852600c49b28a..963af518e6269f2f94a787bb7f5b3356e01b9601 100644 (file)
--- a/folly/io/async/HHWheelTimer.h
+++ b/folly/io/async/HHWheelTimer.h
@@ -16,6 +16,7 @@
 
 #pragma once
 
+#include <folly/Optional.h>
 #include <folly/io/async/AsyncTimeout.h>
 #include <folly/io/async/DelayedDestruction.h>
 
@@ -133,6 +134,7 @@ class HHWheelTimer : private folly::AsyncTimeout,
     void cancelTimeoutImpl();
 
     HHWheelTimer* wheel_;
+    folly::Optional<DestructorGuard> wheelGuard_;
     std::chrono::milliseconds expiration_;
 
     typedef boost::intrusive::list_member_hook<