Commit 6ea3ec05 authored by Stepan Palamarchuk's avatar Stepan Palamarchuk Committed by Facebook Github Bot

Always use real next tick when computing deadline

Summary:
Current logic will use last processed tick as next tick if we are inside of timeout handling.

However, this is wrong in two ways:
* cascading logic would compute number of ticks until expiration from real current time, however it will use `lastTick_` as base and thus may lead to premature timeout.
* if we spent non-trivial amount of time invoking callbacks, we may use stale tick number as base of some timeout.

Reviewed By: djwatson

Differential Revision: D13541504

fbshipit-source-id: b7e675bb5a707161f5c7f636d4c2a374a118da83
parent 67cb8eed
......@@ -148,11 +148,17 @@ void HHWheelTimer::scheduleTimeout(
callback->setScheduled(this, timeout);
auto nextTick = calcNextTick();
// It's possible that the wheel timeout is already scheduled for earlier
// tick, but it didn't run yet. In such case, we must use the lowest of them
// to avoid using the wrong slot.
// There are three possible scenarios:
// - we are currently inside of HHWheelTimer::timeoutExpired. In this case,
// we need to use its lastTick_ as a base for computations
// - HHWheelTimer tick timeout is already scheduled. In this case,
// we need to use its scheduled tick as a base.
// - none of the above are true. In this case, it's safe to use the nextTick
// as a base.
int64_t baseTick = nextTick;
if (isScheduled()) {
if (processingCallbacksGuard_) {
baseTick = std::min(lastTick_, nextTick);
} else if (isScheduled()) {
baseTick = std::min(expireTick_, nextTick);
}
scheduleTimeoutImpl(callback, timeout, baseTick, nextTick);
......@@ -245,7 +251,7 @@ void HHWheelTimer::timeoutExpired() noexcept {
return;
}
}
scheduleNextTimeout(calcNextTick());
scheduleNextTimeout(lastTick_);
}
size_t HHWheelTimer::cancelAll() {
......@@ -319,16 +325,7 @@ size_t HHWheelTimer::cancelTimeoutsFromList(CallbackList& timeouts) {
}
int64_t HHWheelTimer::calcNextTick() {
auto intervals = (getCurTime() - startTime_) / interval_;
// Slow eventbases will have skew between the actual time and the
// callback time. To avoid racing the next scheduleNextTimeout()
// call, always schedule new timeouts against the actual
// timeoutExpired() time.
if (!processingCallbacksGuard_) {
return intervals;
} else {
return lastTick_;
}
return (getCurTime() - startTime_) / interval_;
}
} // namespace folly
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