Commit 2355eee3 authored by Andrii Grynenko's avatar Andrii Grynenko Committed by Alecs King

Make sure we can't access LocalData when destroying it

Summary: We can access LocalData while currentFiber is set. We should make sure it's set to null when LocalData::reset is called.

Test Plan: unit test

Reviewed By: alikhtarov@fb.com

Subscribers: folly-diffs@, yfeldblum, chalfant, bwatling

FB internal diff: D1996071

Tasks: 6725667

Signature: t1:1996071:1429135408:d549d577e140ce2867aff4130e73be3884dbd2ed
parent d3ceb232
...@@ -39,6 +39,11 @@ inline void FiberManager::ensureLoopScheduled() { ...@@ -39,6 +39,11 @@ inline void FiberManager::ensureLoopScheduled() {
} }
inline void FiberManager::runReadyFiber(Fiber* fiber) { inline void FiberManager::runReadyFiber(Fiber* fiber) {
SCOPE_EXIT {
assert(currentFiber_ == nullptr);
assert(activeFiber_ == nullptr);
};
assert(fiber->state_ == Fiber::NOT_STARTED || assert(fiber->state_ == Fiber::NOT_STARTED ||
fiber->state_ == Fiber::READY_TO_RUN); fiber->state_ == Fiber::READY_TO_RUN);
currentFiber_ = fiber; currentFiber_ = fiber;
...@@ -61,6 +66,7 @@ inline void FiberManager::runReadyFiber(Fiber* fiber) { ...@@ -61,6 +66,7 @@ inline void FiberManager::runReadyFiber(Fiber* fiber) {
if (fiber->state_ == Fiber::AWAITING) { if (fiber->state_ == Fiber::AWAITING) {
awaitFunc_(*fiber); awaitFunc_(*fiber);
awaitFunc_ = nullptr; awaitFunc_ = nullptr;
currentFiber_ = nullptr;
} else if (fiber->state_ == Fiber::INVALID) { } else if (fiber->state_ == Fiber::INVALID) {
assert(fibersActive_ > 0); assert(fibersActive_ > 0);
--fibersActive_; --fibersActive_;
...@@ -77,6 +83,8 @@ inline void FiberManager::runReadyFiber(Fiber* fiber) { ...@@ -77,6 +83,8 @@ inline void FiberManager::runReadyFiber(Fiber* fiber) {
} }
fiber->finallyFunc_ = nullptr; fiber->finallyFunc_ = nullptr;
} }
// Make sure LocalData is not accessible from its destructor
currentFiber_ = nullptr;
fiber->localData_.reset(); fiber->localData_.reset();
if (fibersPoolSize_ < options_.maxFibersPoolSize) { if (fibersPoolSize_ < options_.maxFibersPoolSize) {
...@@ -88,10 +96,10 @@ inline void FiberManager::runReadyFiber(Fiber* fiber) { ...@@ -88,10 +96,10 @@ inline void FiberManager::runReadyFiber(Fiber* fiber) {
--fibersAllocated_; --fibersAllocated_;
} }
} else if (fiber->state_ == Fiber::YIELDED) { } else if (fiber->state_ == Fiber::YIELDED) {
currentFiber_ = nullptr;
fiber->state_ = Fiber::READY_TO_RUN; fiber->state_ = Fiber::READY_TO_RUN;
yieldedFibers_.push_back(*fiber); yieldedFibers_.push_back(*fiber);
} }
currentFiber_ = nullptr;
} }
inline bool FiberManager::loopUntilNoReady() { inline bool FiberManager::loopUntilNoReady() {
......
...@@ -1281,6 +1281,32 @@ TEST(FiberManager, fiberLocalHeap) { ...@@ -1281,6 +1281,32 @@ TEST(FiberManager, fiberLocalHeap) {
testFiberLocal<LargeData>(); testFiberLocal<LargeData>();
} }
TEST(FiberManager, fiberLocalDestructor) {
struct CrazyData {
size_t data{42};
~CrazyData() {
if (data == 41) {
addTask([]() {
EXPECT_EQ(42, local<CrazyData>().data);
// Make sure we don't have infinite loop
local<CrazyData>().data = 0;
});
}
}
};
FiberManager fm(LocalType<CrazyData>(),
folly::make_unique<SimpleLoopController>());
fm.addTask([]() {
local<CrazyData>().data = 41;
});
fm.loopUntilNoReady();
EXPECT_FALSE(fm.hasTasks());
}
TEST(FiberManager, yieldTest) { TEST(FiberManager, yieldTest) {
FiberManager manager(folly::make_unique<SimpleLoopController>()); FiberManager manager(folly::make_unique<SimpleLoopController>());
auto& loopController = auto& loopController =
......
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