From d02b0720813a62a4f1fe875e6e784843f5c2da46 Mon Sep 17 00:00:00 2001 From: Dan Engelbrecht Date: Wed, 15 Nov 2023 11:30:56 +0100 Subject: fix race contdition when signaling shutdown of process and waiting for completion (#539) --- src/zenutil/zenserverprocess.cpp | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) (limited to 'src/zenutil/zenserverprocess.cpp') diff --git a/src/zenutil/zenserverprocess.cpp b/src/zenutil/zenserverprocess.cpp index 83c6668ba..00736321c 100644 --- a/src/zenutil/zenserverprocess.cpp +++ b/src/zenutil/zenserverprocess.cpp @@ -468,7 +468,14 @@ ZenServerInstance::ZenServerInstance(ZenServerEnvironment& TestEnvironment) : m_ ZenServerInstance::~ZenServerInstance() { - Shutdown(); + try + { + Shutdown(); + } + catch (const std::exception& Err) + { + ZEN_ERROR("Shutting down zenserver instance failed, reason: '{}'", Err.what()); + } } void @@ -491,6 +498,7 @@ ZenServerInstance::Shutdown() else { SignalShutdown(); + ZEN_DEBUG("Waiting for zenserver process {} to shut down", m_Process.Pid()); m_Process.Wait(); m_Process.Reset(); } -- cgit v1.2.3 From 9f2a7e58fb8a30e630d8b465d2ba832a861579c2 Mon Sep 17 00:00:00 2001 From: Dan Engelbrecht Date: Wed, 15 Nov 2023 12:13:00 +0100 Subject: add doctest listener so we can output when test/subtests begin (#538) * add doctest listener so we can output when test/subtests begin * disable sentry when running a test server --- src/zenutil/zenserverprocess.cpp | 1 + 1 file changed, 1 insertion(+) (limited to 'src/zenutil/zenserverprocess.cpp') diff --git a/src/zenutil/zenserverprocess.cpp b/src/zenutil/zenserverprocess.cpp index 00736321c..3e735dc07 100644 --- a/src/zenutil/zenserverprocess.cpp +++ b/src/zenutil/zenserverprocess.cpp @@ -535,6 +535,7 @@ ZenServerInstance::SpawnServer(int BasePort, std::string_view AdditionalServerAr } CommandLine << " --test --log-id " << LogId; + CommandLine << " --no-sentry"; } if (m_OwnerPid.has_value()) -- cgit v1.2.3 From 573907447db3e19d49c0bcaf3f659cf2d599c738 Mon Sep 17 00:00:00 2001 From: Dan Engelbrecht Date: Thu, 16 Nov 2023 18:50:27 +0100 Subject: blocking queue fix (#550) * make BlockingQueue::m_CompleteAdding non-atomic * ZenCacheDiskLayer::Flush logging * name worker threads in ZenCacheDiskLayer::DiscoverBuckets * name worker threads in gcv2 * improved logging in ZenServerInstance * scrub threadpool naming * remove waitpid handling, we should just call wait to kill zombie processes --- src/zenutil/zenserverprocess.cpp | 46 ++++++++++++++++++++++++++++++---------- 1 file changed, 35 insertions(+), 11 deletions(-) (limited to 'src/zenutil/zenserverprocess.cpp') diff --git a/src/zenutil/zenserverprocess.cpp b/src/zenutil/zenserverprocess.cpp index 3e735dc07..2971b0fab 100644 --- a/src/zenutil/zenserverprocess.cpp +++ b/src/zenutil/zenserverprocess.cpp @@ -487,20 +487,33 @@ ZenServerInstance::SignalShutdown() void ZenServerInstance::Shutdown() { - if (m_Process.IsValid() && m_ShutdownOnDestroy) + if (m_Process.IsValid()) { - if (m_Terminate) + if (m_ShutdownOnDestroy) { - ZEN_INFO("Terminating zenserver process"); - m_Process.Terminate(111); - m_Process.Reset(); + if (m_Terminate) + { + ZEN_INFO("Terminating zenserver process {}", m_Name); + m_Process.Terminate(111); + m_Process.Reset(); + ZEN_DEBUG("zenserver process {} ({}) terminated", m_Name, m_Process.Pid()); + } + else + { + ZEN_DEBUG("Requesting zenserver process {} ({}) to shut down", m_Name, m_Process.Pid()); + SignalShutdown(); + ZEN_DEBUG("Waiting for zenserver process {} ({}) to shut down", m_Name, m_Process.Pid()); + while (!m_Process.Wait(5000)) + { + ZEN_WARN("Waiting for zenserver process {} ({}) timed out", m_Name, m_Process.Pid()); + } + m_Process.Reset(); + } + ZEN_DEBUG("zenserver process {} ({}) exited", m_Name, m_Process.Pid()); } else { - SignalShutdown(); - ZEN_DEBUG("Waiting for zenserver process {} to shut down", m_Process.Pid()); - m_Process.Wait(); - m_Process.Reset(); + ZEN_DEBUG("Detached from zenserver process {} ({})", m_Name, m_Process.Pid()); } } } @@ -521,6 +534,7 @@ ZenServerInstance::SpawnServer(int BasePort, std::string_view AdditionalServerAr ExtendableStringBuilder<32> LogId; LogId << "Zen" << ChildId; + m_Name = LogId.ToString(); ExtendableStringBuilder<512> CommandLine; CommandLine << "zenserver" ZEN_EXE_SUFFIX_LITERAL; // see CreateProc() call for actual binary path @@ -534,7 +548,7 @@ ZenServerInstance::SpawnServer(int BasePort, std::string_view AdditionalServerAr m_OwnerPid = MyPid; } - CommandLine << " --test --log-id " << LogId; + CommandLine << " --test --log-id " << m_Name; CommandLine << " --no-sentry"; } @@ -613,7 +627,7 @@ ZenServerInstance::SpawnServer(int BasePort, std::string_view AdditionalServerAr { if (!WaitUntilReady(WaitTimeoutMs)) { - throw std::runtime_error(fmt::format("server start timeout after {}", NiceTimeSpanMs(WaitTimeoutMs))); + throw std::runtime_error(fmt::format("server start of {} timeout after {}", m_Name, NiceTimeSpanMs(WaitTimeoutMs))); } // Determine effective base port @@ -737,4 +751,14 @@ ZenServerInstance::SetTestDir(std::filesystem::path TestDir) m_TestDir = TestDir; } +bool +ZenServerInstance::IsRunning() +{ + if (!m_Process.IsValid()) + { + return false; + } + return m_Process.IsRunning(); +} + } // namespace zen -- cgit v1.2.3 From 68915961c397c9f06930f80677d131a25d045962 Mon Sep 17 00:00:00 2001 From: Stefan Boberg Date: Fri, 17 Nov 2023 10:30:51 +0100 Subject: use dynamic port assignment for tests (#545) this change replaces hard-coded port numbers in tests with dynamically assigned ports, to avoid potential issues around socket lifetimes and re-use policies --- src/zenutil/zenserverprocess.cpp | 97 +++++++++++++++++++++++----------------- 1 file changed, 57 insertions(+), 40 deletions(-) (limited to 'src/zenutil/zenserverprocess.cpp') diff --git a/src/zenutil/zenserverprocess.cpp b/src/zenutil/zenserverprocess.cpp index 2971b0fab..909692fbc 100644 --- a/src/zenutil/zenserverprocess.cpp +++ b/src/zenutil/zenserverprocess.cpp @@ -12,6 +12,8 @@ #include +#include + #if ZEN_PLATFORM_WINDOWS # include #else @@ -530,8 +532,6 @@ ZenServerInstance::SpawnServer(int BasePort, std::string_view AdditionalServerAr ChildEventName << "Zen_Child_" << ChildId; NamedEvent ChildEvent{ChildEventName}; - CreateShutdownEvent(BasePort); - ExtendableStringBuilder<32> LogId; LogId << "Zen" << ChildId; m_Name = LogId.ToString(); @@ -567,7 +567,7 @@ ZenServerInstance::SpawnServer(int BasePort, std::string_view AdditionalServerAr if (BasePort) { CommandLine << " --port " << BasePort; - m_BasePort = BasePort; + m_BasePort = gsl::narrow_cast(BasePort); } if (!m_TestDir.empty()) @@ -629,39 +629,6 @@ ZenServerInstance::SpawnServer(int BasePort, std::string_view AdditionalServerAr { throw std::runtime_error(fmt::format("server start of {} timeout after {}", m_Name, NiceTimeSpanMs(WaitTimeoutMs))); } - - // Determine effective base port - - ZenServerState State; - if (!State.InitializeReadOnly()) - { - // TODO: return success/error code instead? - throw std::runtime_error("no zen state found"); - } - - const ZenServerState::ZenServerEntry* Entry = nullptr; - - if (BasePort) - { - Entry = State.Lookup(BasePort); - } - else - { - State.Snapshot([&](const ZenServerState::ZenServerEntry& InEntry) { - if (InEntry.Pid == static_cast(GetProcessId(ChildPid))) - { - Entry = &InEntry; - } - }); - } - - if (!Entry) - { - // TODO: return success/error code instead? - throw std::runtime_error("no server entry found"); - } - - m_BasePort = Entry->EffectiveListenPort; } } @@ -717,23 +684,73 @@ ZenServerInstance::Detach() } } -void +uint16_t ZenServerInstance::WaitUntilReady() { while (m_ReadyEvent.Wait(100) == false) { if (!m_Process.IsRunning() || !m_Process.IsValid()) { - ZEN_INFO("Wait abandoned by invalid process (running={})", m_Process.IsRunning()); - return; + ZEN_WARN("Wait abandoned by invalid process (running={})", m_Process.IsRunning()); + + return 0; } } + + OnServerReady(); + + return m_BasePort; } bool ZenServerInstance::WaitUntilReady(int Timeout) { - return m_ReadyEvent.Wait(Timeout); + if (m_ReadyEvent.Wait(Timeout)) + { + OnServerReady(); + + return true; + } + + return false; +} + +void +ZenServerInstance::OnServerReady() +{ + // Determine effective base port + + ZenServerState State; + if (!State.InitializeReadOnly()) + { + // TODO: return success/error code instead? + throw std::runtime_error("no zen state found"); + } + + const ZenServerState::ZenServerEntry* Entry = nullptr; + + if (m_BasePort) + { + Entry = State.Lookup(m_BasePort); + } + else + { + State.Snapshot([&](const ZenServerState::ZenServerEntry& InEntry) { + if (InEntry.Pid == (uint32_t)m_Process.Pid()) + { + Entry = &InEntry; + } + }); + } + + if (!Entry) + { + // TODO: return success/error code instead? + throw std::runtime_error("no server entry found"); + } + + m_BasePort = Entry->EffectiveListenPort; + CreateShutdownEvent(m_BasePort); } std::string -- cgit v1.2.3