From 1e321f0959643f905a3af2505f90c838293f37ca Mon Sep 17 00:00:00 2001 From: Gerd von Egidy Date: Tue, 29 Apr 2014 00:05:39 +0200 Subject: [PATCH] Fix possible race when the reload-process determines it's parent pid and the true parent has ended Now transfers the true parent pid as parameter to --reload-internal. Refs #5788 --- icinga-app/icinga.cpp | 11 ++++++----- lib/base/application.cpp | 7 ++++++- lib/base/utility.cpp | 15 --------------- lib/base/utility.h | 1 - 4 files changed, 12 insertions(+), 22 deletions(-) diff --git a/icinga-app/icinga.cpp b/icinga-app/icinga.cpp index 4208bed6a..8c7940ca1 100644 --- a/icinga-app/icinga.cpp +++ b/icinga-app/icinga.cpp @@ -195,8 +195,8 @@ static bool Daemonize(const String& stderrFile) static void TerminateAndWaitForEnd(pid_t target) { #ifndef _WIN32 - // allow 15 seconds timeout - double timeout = Utility::GetTime() + 15; + // allow 30 seconds timeout + double timeout = Utility::GetTime() + 30; int ret = kill(target, SIGTERM); @@ -279,8 +279,8 @@ int Main(void) ("validate,C", "exit after validating the configuration") ("debug,x", "enable debugging") ("errorlog,e", po::value(), "log fatal errors to the specified log file (only works in combination with --daemonize)") - ("reload-internal", "used internally to implement config reload: do not call manually, send SIGHUP instead") #ifndef _WIN32 + ("reload-internal", po::value(), "used internally to implement config reload: do not call manually, send SIGHUP instead") ("daemonize,d", "detach from the controlling terminal") ("user,u", po::value(), "user to run Icinga as") ("group,g", po::value(), "group to run Icinga as") @@ -450,8 +450,9 @@ int Main(void) } if(g_AppParams.count("reload-internal")) { - Log(LogInformation, "icinga-app", "Terminating previous instance of Icinga (PID " + Convert::ToString(Utility::GetParentPid()) + ")"); - TerminateAndWaitForEnd(Utility::GetParentPid()); + int parentpid = g_AppParams["reload-internal"].as(); + Log(LogInformation, "icinga-app", "Terminating previous instance of Icinga (PID " + Convert::ToString(parentpid) + ")"); + TerminateAndWaitForEnd(parentpid); Log(LogInformation, "icinga-app", "Previous instance has ended, taking over now."); } diff --git a/lib/base/application.cpp b/lib/base/application.cpp index 6cef2b83f..5a5f36757 100644 --- a/lib/base/application.cpp +++ b/lib/base/application.cpp @@ -26,6 +26,7 @@ #include "base/utility.h" #include "base/debug.h" #include "base/type.h" +#include "base/convert.h" #include "base/scriptvariable.h" #include "icinga-version.h" #include @@ -282,10 +283,14 @@ void Application::StartReloadProcess(void) const std::vector args; args.push_back(GetExePath(m_ArgV[0])); - for (int i=1; i < Application::GetArgC(); i++) + for (int i=1; i < Application::GetArgC(); i++) { if (std::string(Application::GetArgV()[i]) != "--reload-internal") args.push_back(Application::GetArgV()[i]); + else + i++; // the next parameter after --reload-internal is the pid, remove that too + } args.push_back("--reload-internal"); + args.push_back(Convert::ToString(Utility::GetPid())); Process::Ptr process = make_shared(args); diff --git a/lib/base/utility.cpp b/lib/base/utility.cpp index dc3c8232b..56559fea2 100644 --- a/lib/base/utility.cpp +++ b/lib/base/utility.cpp @@ -331,21 +331,6 @@ pid_t Utility::GetPid(void) #endif /* _WIN32 */ } -/** - * Returns the ID of the parent process. - * - * @returns The PID. - */ -pid_t Utility::GetParentPid(void) -{ -#ifndef _WIN32 - return getppid(); -#else /* _WIN32 */ - // TODO - return 0; -#endif /* _WIN32 */ -} - /** * Sleeps for the specified amount of time. * diff --git a/lib/base/utility.h b/lib/base/utility.h index ada33d43a..f8c05a5fe 100644 --- a/lib/base/utility.h +++ b/lib/base/utility.h @@ -74,7 +74,6 @@ public: static double GetTime(void); static pid_t GetPid(void); - static pid_t GetParentPid(void); static void Sleep(double timeout);