zenserverprocess hardening (#61)

* verify running process before creating event
* make sure we don't signal/wait for a zenserver instance that we did not wait for to get ready

1 files changed, 28 insertions, 9 deletions

diff --git a/src/zenutil/zenserverprocess.cpp b/src/zenutil/zenserverprocess.cpp
index 34eec9790..26c72fc9b 100644
--- a/src/zenutil/zenserverprocess.cpp
+++ b/src/zenutil/zenserverprocess.cpp
@@ -499,7 +499,7 @@ ZenServerEnvironment::CreateNewTestDir()
 	using namespace std::literals;
 
 	ExtendableWideStringBuilder<256> TestDir;
-	TestDir << "test"sv << int64_t(++ZenServerTestCounter);
+	TestDir << "test"sv << int64_t(ZenServerTestCounter.fetch_add(1));
 
 	std::filesystem::path TestPath = m_TestBaseDir / TestDir.c_str();
 	ZEN_ASSERT(!std::filesystem::exists(TestPath));
@@ -544,10 +544,18 @@ ZenServerInstance::~ZenServerInstance()
 	}
 }
 
-void
+bool
 ZenServerInstance::SignalShutdown()
 {
-	m_ShutdownEvent.Set();
+	if (m_ShutdownEvent)
+	{
+		m_ShutdownEvent->Set();
+		return true;
+	}
+	else
+	{
+		return false;
+	}
 }
 
 int
@@ -562,7 +570,6 @@ ZenServerInstance::Shutdown()
 				ZEN_INFO("Terminating zenserver process {}", m_Name);
 				int ExitCode = 111;
 				m_Process.Terminate(ExitCode);
-				m_Process.Reset();
 				ZEN_DEBUG("zenserver process {} ({}) terminated", m_Name, m_Process.Pid());
 				return ExitCode;
 			}
@@ -571,9 +578,16 @@ ZenServerInstance::Shutdown()
 				if (m_Process.IsRunning())
 				{
 					ZEN_DEBUG("Requesting zenserver process {} ({}) to shut down", m_Name, m_Process.Pid());
-					SignalShutdown();
+					if (!SignalShutdown())
+					{
+						ZEN_INFO("Terminating zenserver process as we did not wait for it to get ready {}", m_Name);
+						int ExitCode = 111;
+						m_Process.Terminate(ExitCode);
+						ZEN_DEBUG("zenserver process {} ({}) terminated", m_Name, m_Process.Pid());
+						return ExitCode;
+					}
 					ZEN_DEBUG("Waiting for zenserver process {} ({}) to shut down", m_Name, m_Process.Pid());
-					while (!m_Process.Wait(5000))
+					while (!m_Process.Wait(1000))
 					{
 						ZEN_WARN("Waiting for zenserver process {} ({}) timed out", m_Name, m_Process.Pid());
 					}
@@ -720,8 +734,7 @@ ZenServerInstance::CreateShutdownEvent(int BasePort)
 	ExtendableStringBuilder<32> ChildShutdownEventName;
 	ChildShutdownEventName << "Zen_" << BasePort;
 	ChildShutdownEventName << "_Shutdown";
-	NamedEvent ChildShutdownEvent{ChildShutdownEventName};
-	m_ShutdownEvent = std::move(ChildShutdownEvent);
+	m_ShutdownEvent = std::make_unique<NamedEvent>(ChildShutdownEventName);
 }
 
 void
@@ -774,7 +787,7 @@ ZenServerInstance::Detach()
 	if (m_Process.IsValid())
 	{
 		m_Process.Reset();
-		m_ShutdownEvent.Close();
+		m_ShutdownEvent.reset();
 	}
 }
 
@@ -862,8 +875,14 @@ ZenServerInstance::OnServerReady()
 		// TODO: return success/error code instead?
 		throw std::runtime_error("no server entry found");
 	}
+	ZEN_ASSERT(Entry->IsReady());
 
 	m_BasePort = Entry->EffectiveListenPort;
+	ZEN_ASSERT(m_BasePort != 0);
+	if (!IsProcessRunning(Entry->Pid.load()))
+	{
+		throw std::runtime_error("server no longer running");
+	}
 	CreateShutdownEvent(m_BasePort);
 
 	ZEN_DEBUG("Server '{}' is ready on port {}", m_Name, m_BasePort);