[mesos] branch master updated: Fixed a bug where timers wouldn't expire after `process:reinitialize`.

[bmahler]

This is an automated email from the ASF dual-hosted git repository.

bmahler pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/mesos.git


The following commit(s) were added to refs/heads/master by this push:
     new 72f16f6  Fixed a bug where timers wouldn't expire after 
`process:reinitialize`.
72f16f6 is described below

commit 72f16f68973bf7d2ce5c621539a21fc4eccfa56e
Author: Charles-Francois Natali <cf.nat...@gmail.com>
AuthorDate: Sat Jun 26 19:04:33 2021 +0100

    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.
---
 3rdparty/libprocess/src/clock.cpp               | 10 ++++---
 3rdparty/libprocess/src/tests/process_tests.cpp | 37 +++++++++++++++++++++++++
 2 files changed, 43 insertions(+), 4 deletions(-)

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 @@ void Clock::finalize()
   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::InvokeWithoutArgs;
 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 @@ TEST_F(ProcessTest, ProcessesEndpointNoHang)
   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);
+}