Commit 06b04cb1 authored by Jon Maltiel Swenson's avatar Jon Maltiel Swenson Committed by facebook-github-bot-9

Cancel timeout only if not run

Summary: Do not cancel TimeoutHandler timeout if the timeout has already run.

Reviewed By: spalamarchuk

Differential Revision: D2622280

fb-gh-sync-id: 27d83b44ab225e2859695f4e5f21cef934824a35
parent 131db120
...@@ -27,8 +27,14 @@ void Baton::wait() { ...@@ -27,8 +27,14 @@ void Baton::wait() {
} }
void Baton::wait(TimeoutHandler& timeoutHandler) { void Baton::wait(TimeoutHandler& timeoutHandler) {
timeoutHandler.setBaton(this); auto timeoutFunc = [this, &timeoutHandler] {
timeoutHandler.setFiberManager(FiberManager::getFiberManagerUnsafe()); if (!try_wait()) {
postHelper(TIMEOUT);
}
timeoutHandler.timeoutPtr_ = 0;
};
timeoutHandler.timeoutFunc_ = std::ref(timeoutFunc);
timeoutHandler.fiberManager_ = FiberManager::getFiberManagerUnsafe();
wait(); wait();
timeoutHandler.cancelTimeout(); timeoutHandler.cancelTimeout();
} }
...@@ -163,17 +169,15 @@ void Baton::reset() { ...@@ -163,17 +169,15 @@ void Baton::reset() {
waitingFiber_.store(NO_WAITER, std::memory_order_relaxed);; waitingFiber_.store(NO_WAITER, std::memory_order_relaxed);;
} }
void Baton::TimeoutHandler::scheduleTimeout(uint32_t timeoutMs) { void Baton::TimeoutHandler::scheduleTimeout(
TimeoutController::Duration timeout) {
assert(fiberManager_ != nullptr); assert(fiberManager_ != nullptr);
assert(baton_ != nullptr); assert(timeoutFunc_ != nullptr);
if (timeoutMs > 0) { assert(timeoutPtr_ == 0);
if (timeout.count() > 0) {
timeoutPtr_ = fiberManager_->timeoutManager_->registerTimeout( timeoutPtr_ = fiberManager_->timeoutManager_->registerTimeout(
[baton = baton_]() { timeoutFunc_, timeout);
if (!baton->try_wait()) {
baton->postHelper(TIMEOUT);
}
},
std::chrono::milliseconds(timeoutMs));
} }
} }
......
...@@ -113,28 +113,22 @@ class Baton { ...@@ -113,28 +113,22 @@ class Baton {
/** /**
* Provides a way to schedule a wakeup for a wait()ing fiber. * Provides a way to schedule a wakeup for a wait()ing fiber.
* A TimeoutHandler must be passed to Baton::wait(TimeoutHandler&) * A TimeoutHandler must be passed to Baton::wait(TimeoutHandler&)
* before timeouts are scheduled/cancelled. It is only safe to use the * before a timeout is scheduled. It is only safe to use the
* TimeoutHandler on the same thread as the wait()ing fiber. * TimeoutHandler on the same thread as the wait()ing fiber.
* scheduleTimeout() may only be called once prior to the end of the * scheduleTimeout() may only be called once prior to the end of the
* associated Baton's life. * associated Baton's life.
*/ */
class TimeoutHandler { class TimeoutHandler {
public: public:
void scheduleTimeout(uint32_t timeoutMs); void scheduleTimeout(TimeoutController::Duration timeoutMs);
void cancelTimeout();
private: private:
friend class Baton; friend class Baton;
void setFiberManager(FiberManager* fiberManager) { void cancelTimeout();
fiberManager_ = fiberManager;
}
void setBaton(Baton* baton) {
baton_ = baton;
}
std::function<void()> timeoutFunc_{nullptr};
FiberManager* fiberManager_{nullptr}; FiberManager* fiberManager_{nullptr};
Baton* baton_{nullptr};
intptr_t timeoutPtr_{0}; intptr_t timeoutPtr_{0};
}; };
......
...@@ -1485,7 +1485,7 @@ TEST(FiberManager, batonWaitTimeoutHandler) { ...@@ -1485,7 +1485,7 @@ TEST(FiberManager, batonWaitTimeoutHandler) {
EXPECT_FALSE(baton.try_wait()); EXPECT_FALSE(baton.try_wait());
EXPECT_EQ(0, fibersRun); EXPECT_EQ(0, fibersRun);
timeoutHandler.scheduleTimeout(250); timeoutHandler.scheduleTimeout(std::chrono::milliseconds(250));
std::this_thread::sleep_for(std::chrono::milliseconds(500)); std::this_thread::sleep_for(std::chrono::milliseconds(500));
EXPECT_FALSE(baton.try_wait()); EXPECT_FALSE(baton.try_wait());
...@@ -1497,6 +1497,36 @@ TEST(FiberManager, batonWaitTimeoutHandler) { ...@@ -1497,6 +1497,36 @@ TEST(FiberManager, batonWaitTimeoutHandler) {
EXPECT_EQ(1, fibersRun); EXPECT_EQ(1, fibersRun);
} }
TEST(FiberManager, batonWaitTimeoutMany) {
FiberManager manager(folly::make_unique<EventBaseLoopController>());
folly::EventBase evb;
dynamic_cast<EventBaseLoopController&>(manager.loopController())
.attachEventBase(evb);
constexpr size_t kNumTimeoutTasks = 10000;
size_t tasksCount = kNumTimeoutTasks;
// We add many tasks to hit timeout queue deallocation logic.
for (size_t i = 0; i < kNumTimeoutTasks; ++i) {
manager.addTask([&]() {
Baton baton;
Baton::TimeoutHandler timeoutHandler;
folly::fibers::addTask([&] {
timeoutHandler.scheduleTimeout(std::chrono::milliseconds(1000));
});
baton.wait(timeoutHandler);
if (--tasksCount == 0) {
evb.terminateLoopSoon();
}
});
}
evb.loopForever();
}
static size_t sNumAwaits; static size_t sNumAwaits;
void runBenchmark(size_t numAwaits, size_t toSend) { void runBenchmark(size_t numAwaits, size_t toSend) {
......
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