mesos git commit: Libprocess Reinitialization: Implemented Clock::finalize.

[jo...@apache.org]

Repository: mesos
Updated Branches:
  refs/heads/master 800860578 -> bde3b2a47


Libprocess Reinitialization: Implemented Clock::finalize.

Timers may deference `process_manager`, either directly or via the
associated lambda, after `process_manager` has been freed in
`process::finalize`.  Cleaning up the `Clock` appropriately in
`process::finalize` prevents that race.

Review: https://reviews.apache.org/r/40264


Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/bde3b2a4
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/bde3b2a4
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/bde3b2a4

Branch: refs/heads/master
Commit: bde3b2a47ccd100cf37d2b718ea3fde81bd5b091
Parents: 8008605
Author: Joseph Wu <jo...@mesosphere.io>
Authored: Mon Nov 23 14:59:24 2015 -0500
Committer: Joris Van Remoortere <jo...@gmail.com>
Committed: Mon Nov 23 15:11:01 2015 -0500

----------------------------------------------------------------------
 3rdparty/libprocess/include/process/clock.hpp | 11 +++++++++++
 3rdparty/libprocess/src/clock.cpp             | 21 +++++++++++++++++++++
 3rdparty/libprocess/src/process.cpp           | 15 +++++++++++++--
 3 files changed, 45 insertions(+), 2 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/bde3b2a4/3rdparty/libprocess/include/process/clock.hpp
----------------------------------------------------------------------
diff --git a/3rdparty/libprocess/include/process/clock.hpp b/3rdparty/libprocess/include/process/clock.hpp
index 2e526f8..5875847 100644
--- a/3rdparty/libprocess/include/process/clock.hpp
+++ b/3rdparty/libprocess/include/process/clock.hpp
@@ -48,6 +48,17 @@ public:
       lambda::function<void(const std::list<Timer>&)>&& callback);
 
   /**
+   * Clears all timers without executing them.
+   *
+   * The process manager must be properly finalized before the clock is
+   * finalized.  This will eliminate the need for timers to activate and
+   * prevent further timers from being added after finalization.
+   *
+   * Also, the Clock must not be paused when finalizing.
+   */
+  static void finalize();
+
+  /**
    * The current clock time for either the current process that makes
    * this call or the global clock time if not invoked from a process.
    *

http://git-wip-us.apache.org/repos/asf/mesos/blob/bde3b2a4/3rdparty/libprocess/src/clock.cpp
----------------------------------------------------------------------
diff --git a/3rdparty/libprocess/src/clock.cpp b/3rdparty/libprocess/src/clock.cpp
index 7665d12..124d527 100644
--- a/3rdparty/libprocess/src/clock.cpp
+++ b/3rdparty/libprocess/src/clock.cpp
@@ -136,6 +136,9 @@ void scheduleTick(const map<Time, list<Timer>>& timers, set<Time>* ticks)
 }
 
 
+// NOTE: This method must remain robust to arbitrary invocations.
+// i.e. `tick` should not make any assumptions of what is held in `timers`,
+// which can be empty or have timers that trigger later than the current time.
 void tick(const Time& time)
 {
   list<Timer> timedout;
@@ -206,6 +209,24 @@ void Clock::initialize(lambda::function<void(const list<Timer>&)>&& callback)
 }
 
 
+void Clock::finalize()
+{
+  // We keep track of state differently when the clock is paused.
+  // Finalization only handles cleanup of a running clock.
+  CHECK(!clock::paused) << "Clock must not be paused when finalizing";
+
+  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.
+    timers->clear();
+  }
+}
+
+
 Time Clock::now()
 {
   return now(__process__);

http://git-wip-us.apache.org/repos/asf/mesos/blob/bde3b2a4/3rdparty/libprocess/src/process.cpp
----------------------------------------------------------------------
diff --git a/3rdparty/libprocess/src/process.cpp b/3rdparty/libprocess/src/process.cpp
index 0fa15dd..a7af467 100644
--- a/3rdparty/libprocess/src/process.cpp
+++ b/3rdparty/libprocess/src/process.cpp
@@ -938,12 +938,23 @@ void initialize(const string& delegate)
 }
 
 
+// Gracefully winds down libprocess in roughly the reverse order of
+// initialization.
 void finalize()
 {
+  // The clock is only paused during tests.  Pausing may lead to infinite waits
+  // during clean up, so we make sure the clock is running normally.
+  Clock::resume();
+
+  // This will terminate any existing processes created via `spawn()`,
+  // like `gc`, `help`, `Logging()`, `Profiler()`, and `System()`.
+  // NOTE: This will also stop the event loop.
   delete process_manager;
+  process_manager = NULL;
 
-  // TODO(benh): Finalize/shutdown Clock so that it doesn't attempt
-  // to dereference 'process_manager' in the 'timedout' callback.
+  // The clock must be cleaned up after the `process_manager` as processes
+  // may otherwise add timers after cleaning up.
+  Clock::finalize();
 }