[7/7] mesos git commit: Ensure GC is terminated last during libprocess shutdown.

[jo...@apache.org]

Ensure GC is terminated last during libprocess shutdown.

Without this change, `finalize()` terminates processes in the order
that they happen to be found when iterating over the `processes`
map. That means that if the GarbageCollector process is terminated
while any GC-managed processes are still running, those processes
will not be GC'd (i.e., they will be leaked).

Fix this by skipping the garbage collector process when iterating
over `processes` in `finalize()`, and then only terminating it after
all other processes have been terminated.

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


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

Branch: refs/heads/master
Commit: 0a4b284ef308d3f7c7aeec2210e218cac2de528b
Parents: 472fce7
Author: Neil Conway <ne...@gmail.com>
Authored: Wed Jul 13 11:36:15 2016 -0700
Committer: Joseph Wu <jo...@apache.org>
Committed: Wed Jul 13 13:21:07 2016 -0700

----------------------------------------------------------------------
 3rdparty/libprocess/src/process.cpp | 51 ++++++++++++++++++++++----------
 1 file changed, 35 insertions(+), 16 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/0a4b284e/3rdparty/libprocess/src/process.cpp
----------------------------------------------------------------------
diff --git a/3rdparty/libprocess/src/process.cpp b/3rdparty/libprocess/src/process.cpp
index 181b27d..9661386 100644
--- a/3rdparty/libprocess/src/process.cpp
+++ b/3rdparty/libprocess/src/process.cpp
@@ -1101,9 +1101,6 @@ void finalize()
   // TODO(neilc): We currently don't cleanup or deallocate the
   // socket_manager (MESOS-3910).
 
-  delete gc;
-  gc = nullptr;
-
   // The clock must be cleaned up after the `process_manager` as processes
   // may otherwise add timers after cleaning up.
   Clock::finalize();
@@ -2305,22 +2302,44 @@ ProcessManager::ProcessManager(const Option<string>& _delegate)
 
 ProcessManager::~ProcessManager()
 {
-  ProcessBase* process = nullptr;
-  // Terminate the first process in the queue. Events are deleted
-  // and the process is erased in ProcessManager::cleanup(). Don't
-  // hold the lock or process the whole map as terminating one process
-  // might trigger other terminations. Deal with them one at a time.
-  do {
+  CHECK(gc != nullptr);
+
+  // Terminate one process at a time. Events are deleted and the process
+  // is erased from `processes` in ProcessManager::cleanup(). Don't hold
+  // the lock or process the whole map as terminating one process might
+  // trigger other terminations.
+  //
+  // We skip the GC process here and instead terminate it below. This
+  // ensures that the GC process is running whenever we terminate any
+  // GC-managed process, which is necessary to ensure GC is performed.
+  while (true) {
+    ProcessBase* process = nullptr;
+
     synchronized (processes_mutex) {
-      process = !processes.empty() ? processes.begin()->second : nullptr;
+      foreachvalue (ProcessBase* candidate, processes) {
+        if (candidate == gc) {
+          continue;
+        }
+        process = candidate;
+        break;
+      }
     }
-    if (process != nullptr) {
-      // Terminate this process but do not inject the message,
-      // i.e. allow it to finish its work first.
-      process::terminate(process, false);
-      process::wait(process);
+
+    if (process == nullptr) {
+      break;
     }
-  } while (process != nullptr);
+
+    // Terminate this process but do not inject the message,
+    // i.e. allow it to finish its work first.
+    process::terminate(process, false);
+    process::wait(process);
+  }
+
+  process::terminate(gc, false);
+  process::wait(gc);
+
+  delete gc;
+  gc = nullptr;
 
   // Send signal to all processing threads to stop running.
   joining_threads.store(true);