Fixed a bug where timers wouldn't expire after `process:reinitialize`.
Pending `ticks` are used by `scheduleTick` to decide whether to schedule
an event loop tick when a new timer is scheduled - since we only need to
schedule the event loop tick if the new timer is supposed to expire
earlier than the current earliest timer.
Unfortunately `Clock::finalize` didn't clear `ticks`, which means that
the following could happen:
- schedule a timer T0 for expiration at time t0
- call `process::reinitalize`, which calls `Clock::finalize` but doesn't
clear `ticks`
- schedule a new timer T1 for expiration at time t1 > t0: since
`scheduleTick` would see that there was already the earlier pending
tick for T0 in `ticks` with t0 < t1, it wouldn't actually schedule a
tick of the event loop
Therefore new timers would never fire again.
This caused e.g.
`DockerContainerizerIPv6Test.ROOT_DOCKER_LaunchIPv6HostNetwork` test to
hang since it called `process::reinitialized` while having some active
timers - e.g. the reaper periodic timer.
Also added a test specifically for this bug.
diff --git a/3rdparty/libprocess/src/clock.cpp b/3rdparty/libprocess/src/clock.cpp
index 519c69e..7307de9 100644
--- a/3rdparty/libprocess/src/clock.cpp
+++ b/3rdparty/libprocess/src/clock.cpp
@@ -218,11 +218,13 @@
synchronized (timers_mutex) {
// NOTE: `currents` is only non-empty when the clock is paused.
- // This, along with the `timers_mutex`, is all that is required to clean
- // up any pending timers. Timers are triggered via "ticks". However,
- // we do not need to clear `ticks` because a "tick" with an empty `timers`
- // map will effectively be a no-op.
+ // Clear both timers and ticks.
+ // Note that we need to clear `ticks` as well because the earliest tick in
+ // `ticks` is used by `scheduleTick` to decide whether to schedule an event
+ // loop tick when a new timer is added, so not clearing `ticks` could
+ // cause, after reinitialization, new timers to never fire.
timers->clear();
+ clock::ticks->clear();
}
}
diff --git a/3rdparty/libprocess/src/tests/process_tests.cpp b/3rdparty/libprocess/src/tests/process_tests.cpp
index eabd70e..d47c19b 100644
--- a/3rdparty/libprocess/src/tests/process_tests.cpp
+++ b/3rdparty/libprocess/src/tests/process_tests.cpp
@@ -114,6 +114,17 @@
using testing::Return;
using testing::ReturnArg;
+
+namespace process {
+
+// Used by TimerAfterReinitialize.
+void reinitialize(
+ const Option<string>& delegate,
+ const Option<string>& readonlyAuthenticationRealm,
+ const Option<string>& readwriteAuthenticationRealm);
+
+}
+
// TODO(bmahler): Move tests into their own files as appropriate.
class ProcessTest : public TemporaryDirectoryTest {};
@@ -2109,3 +2120,29 @@
AWAIT_READY(response);
EXPECT_EQ(http::Status::OK, response->code);
}
+
+
+// Test for a bug where timers wouldn't be handled after libprocess was
+// reinitialized.
+TEST_F(ProcessTest, TimerAfterReinitialize)
+{
+ // Schedule a timer, which won't have time to expire before...
+ process::Timer timer_before = Clock::timer(Milliseconds(10), []() {});
+
+ // We reinitialize libprocess.
+ process::reinitialize(
+ None(),
+ process::READWRITE_HTTP_AUTHENTICATION_REALM,
+ process::READONLY_HTTP_AUTHENTICATION_REALM);
+
+ // Now, schedule a new timer which is supposed to fire later than the one
+ // above.
+ Promise<Nothing> promise;
+ Future<Nothing> future = promise.future();
+
+ process::Timer timer_after = Clock::timer(Milliseconds(10),
+ [&]() { promise.set(Nothing()); });
+
+ // Wait until it fires.
+ AWAIT_READY(future);
+}
