mesos git commit: Fixed memory leak in agent signal handlers.

[jo...@apache.org]

Repository: mesos
Updated Branches:
  refs/heads/master a86a14577 -> 4a703b911


Fixed memory leak in agent signal handlers.

This leak was introduced on purpose in:
https://reviews.apache.org/r/34016/

When we run tests in repetition, each agent will create a new signal
handler and leak the old one.  This patch frees the old signal handler
and fixes up some related comments, because the behavior of calling the
signal handler helper multiple times is predictable.

Also fixes a related typo.

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


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

Branch: refs/heads/master
Commit: 4a703b91151568e21b8772af51c22e23b105313c
Parents: a86a145
Author: Joseph Wu <jo...@mesosphere.io>
Authored: Tue Aug 16 12:51:33 2016 -0700
Committer: Joseph Wu <jo...@apache.org>
Committed: Tue Aug 16 12:51:59 2016 -0700

----------------------------------------------------------------------
 src/slave/posix_signalhandler.hpp | 13 ++++++++++---
 src/slave/slave.cpp               |  2 +-
 src/slave/windows_ctrlhandler.hpp | 13 ++++++++++---
 3 files changed, 21 insertions(+), 7 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/4a703b91/src/slave/posix_signalhandler.hpp
----------------------------------------------------------------------
diff --git a/src/slave/posix_signalhandler.hpp b/src/slave/posix_signalhandler.hpp
index 4a54117..49692b1 100644
--- a/src/slave/posix_signalhandler.hpp
+++ b/src/slave/posix_signalhandler.hpp
@@ -18,9 +18,10 @@
 namespace os {
 namespace internal {
 
-// Not using extern as this is used only by the executable. The signal handler
-// should be configured once. Calling it multiple time or from multiple thread
-// has undefined behavior.
+// Not using extern as this is used only by the executable. The signal
+// handler should be configured once. Configuring it multiple times will
+// overwrite any previous handlers. Configuring from multiple threads
+// simultaneously has undefined behavior.
 std::function<void(int, int)>* signaledWrapper = nullptr;
 
 void signalHandler(int sig, siginfo_t* siginfo, void* context)
@@ -33,6 +34,12 @@ void signalHandler(int sig, siginfo_t* siginfo, void* context)
 
 int configureSignal(const std::function<void(int, int)>* signal)
 {
+  // NOTE: We only expect this function to be called called multiple
+  // times inside tests and `mesos-local`.
+  if (signaledWrapper != nullptr) {
+    delete signaledWrapper;
+  }
+
   struct sigaction action;
   memset(&action, 0, sizeof(struct sigaction));
 

http://git-wip-us.apache.org/repos/asf/mesos/blob/4a703b91/src/slave/slave.cpp
----------------------------------------------------------------------
diff --git a/src/slave/slave.cpp b/src/slave/slave.cpp
index 0feb5c5..3688f42 100644
--- a/src/slave/slave.cpp
+++ b/src/slave/slave.cpp
@@ -745,7 +745,7 @@ void Slave::initialize()
 #ifdef __WINDOWS__
   if (!os::internal::installCtrlHandler(&signalHandler)) {
     EXIT(EXIT_FAILURE)
-      << "Failed to configure consoel handlers: " << WindowsError().message;
+      << "Failed to configure console handlers: " << WindowsError().message;
   }
 #else
   if (os::internal::configureSignal(&signalHandler) < 0) {

http://git-wip-us.apache.org/repos/asf/mesos/blob/4a703b91/src/slave/windows_ctrlhandler.hpp
----------------------------------------------------------------------
diff --git a/src/slave/windows_ctrlhandler.hpp b/src/slave/windows_ctrlhandler.hpp
index 9481e0f..25b14a4 100644
--- a/src/slave/windows_ctrlhandler.hpp
+++ b/src/slave/windows_ctrlhandler.hpp
@@ -20,9 +20,10 @@ namespace internal {
 
 #define SIGUSR1 100
 
-// Not using extern as this is used only by the executable. The signal handler
-// should be configured once. Calling it multiple time or from multiple thread
-// has undefined behavior.
+// Not using extern as this is used only by the executable. The signal
+// handler should be configured once. Configuring it multiple times will
+// overwrite any previous handlers. Configuring from multiple threads
+// simultaneously has undefined behavior.
 std::function<void(int, int)>* signaledWrapper = nullptr;
 
 BOOL WINAPI CtrlHandler(DWORD fdwCtrlType)
@@ -48,6 +49,12 @@ BOOL WINAPI CtrlHandler(DWORD fdwCtrlType)
 
 int installCtrlHandler(const std::function<void(int, int)>* signal)
 {
+  // NOTE: We only expect this function to be called called multiple
+  // times inside tests and `mesos-local`.
+  if (signaledWrapper != nullptr) {
+    delete signaledWrapper;
+  }
+
   if (signal != nullptr) {
     signaledWrapper = new std::function<void(int, int)>(*signal);
     return SetConsoleCtrlHandler(CtrlHandler, TRUE);