Commit 9933f34f authored by Andrii Grynenko's avatar Andrii Grynenko Committed by Facebook Github Bot

Fix destruction race-condition in ThreadWheelTimekeeper

Summary: It was possible for interruptHandler() to be called concurrently with the ThreadWheelTimekeeper shutdown resulting in use-after-free when trying to schedule work on the EventBase.

Reviewed By: yfeldblum

Differential Revision: D9948555

fbshipit-source-id: 4eff3cce0488778559dffc47bfc6eb59a65bb203
parent 268c812c
...@@ -56,10 +56,11 @@ struct WTCallback : public std::enable_shared_from_this<WTCallback>, ...@@ -56,10 +56,11 @@ struct WTCallback : public std::enable_shared_from_this<WTCallback>,
} }
protected: protected:
EventBase* base_; folly::Synchronized<EventBase*> base_;
Promise<Unit> promise_; Promise<Unit> promise_;
void timeoutExpired() noexcept override { void timeoutExpired() noexcept override {
base_ = nullptr;
// Don't need Promise anymore, break the circular reference // Don't need Promise anymore, break the circular reference
auto promise = stealPromise(); auto promise = stealPromise();
if (!promise.isFulfilled()) { if (!promise.isFulfilled()) {
...@@ -68,6 +69,7 @@ struct WTCallback : public std::enable_shared_from_this<WTCallback>, ...@@ -68,6 +69,7 @@ struct WTCallback : public std::enable_shared_from_this<WTCallback>,
} }
void callbackCanceled() noexcept override { void callbackCanceled() noexcept override {
base_ = nullptr;
// Don't need Promise anymore, break the circular reference // Don't need Promise anymore, break the circular reference
auto promise = stealPromise(); auto promise = stealPromise();
if (!promise.isFulfilled()) { if (!promise.isFulfilled()) {
...@@ -76,12 +78,13 @@ struct WTCallback : public std::enable_shared_from_this<WTCallback>, ...@@ -76,12 +78,13 @@ struct WTCallback : public std::enable_shared_from_this<WTCallback>,
} }
void interruptHandler(exception_wrapper ew) { void interruptHandler(exception_wrapper ew) {
auto rBase = base_.rlock();
// Capture shared_ptr of self in lambda, if we don't do this, object // Capture shared_ptr of self in lambda, if we don't do this, object
// may go away before the lambda is executed from event base thread. // may go away before the lambda is executed from event base thread.
// This is not racing with timeoutExpired anymore because this is called // This is not racing with timeoutExpired anymore because this is called
// through Future, which means Core is still alive and keeping a ref count // through Future, which means Core is still alive and keeping a ref count
// on us, so what timeouExpired is doing won't make the object go away // on us, so what timeouExpired is doing won't make the object go away
base_->runInEventBaseThread( (*rBase)->runInEventBaseThread(
[me = shared_from_this(), ew = std::move(ew)]() mutable { [me = shared_from_this(), ew = std::move(ew)]() mutable {
me->cancelTimeout(); me->cancelTimeout();
// Don't need Promise anymore, break the circular reference // Don't need Promise anymore, break the circular reference
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment