From 30d8a3d6880ed3c18c3b4cd49debf832a6bd6902 Mon Sep 17 00:00:00 2001 From: Louis Brandy Date: Fri, 10 Apr 2015 17:17:37 -0700 Subject: [PATCH] fix race in ~ThreadWheelTimekeeper Summary: During destruction, we enqueue a callback to `eventBase_` that references `wheelTimer_`. So long as that callback exists, we have a circular dependency as both reference the other and we -cannot- safely destroy this object (and its members). The fix is to break this dependency someway. In this case, I've chosen to simply `AndWait` until the eventBase has consumed the message referencing the timer. From here normal destruction of members can proceed safely (destorying HHWheelTimer first, EventBase second). The problem in the current code is that the Eventbase will attempt to consume all messages during it's own destruction. If `wheelTimer_->cancelAll()` is still enqueued, it will attempt to use the now destroyed `wheelTimer_`. Test Plan: Running this test repeatedly was able to repro: ./folly/futures/futures-test --gtest_filter=Timekeeper.futureWithinVoidSpecialization Reviewed By: yfeldblum@fb.com Subscribers: enis, folly-diffs@, jsedgwick, yfeldblum, darshan, chalfant FB internal diff: D1985967 Tasks: 6332729, 6741095 Signature: t1:1985967:1428726270:997ec277c6a73554e54b8cf673acd36ff62976e6 --- folly/futures/detail/ThreadWheelTimekeeper.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/folly/futures/detail/ThreadWheelTimekeeper.cpp b/folly/futures/detail/ThreadWheelTimekeeper.cpp index b21be18a..51965d10 100644 --- a/folly/futures/detail/ThreadWheelTimekeeper.cpp +++ b/folly/futures/detail/ThreadWheelTimekeeper.cpp @@ -70,7 +70,7 @@ ThreadWheelTimekeeper::ThreadWheelTimekeeper() : } ThreadWheelTimekeeper::~ThreadWheelTimekeeper() { - eventBase_.runInEventBaseThread([this]{ + eventBase_.runInEventBaseThreadAndWait([this]{ wheelTimer_->cancelAll(); }); eventBase_.terminateLoopSoon(); -- 2.34.1