aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorStefan Boberg <[email protected]>2026-04-21 18:28:11 +0200
committerGitHub Enterprise <[email protected]>2026-04-21 18:28:11 +0200
commit7136132c4c3ae52becab525bc4ce30f3f36126a9 (patch)
treecc6f5079abd0e0178cb7f3ae4a871d14e16a1682
parent5.8.6-pre0 (diff)
downloadarchived-zen-7136132c4c3ae52becab525bc4ce30f3f36126a9.tar.xz
archived-zen-7136132c4c3ae52becab525bc4ce30f3f36126a9.zip
Fix Windows service shutdown signalling (#999)
Stopping the zenserver Windows service (via `sc stop`, `zen service stop`, system shutdown, or any other SCM path) was being ignored. SCM would eventually force-kill the process after its timeout, giving an ungraceful shutdown. ## Root cause PR #751 ("add simple http client tests", c37421a3b) restructured each HTTP server's `OnRun` loop from ```cpp do { m_ShutdownEvent.Wait(WaitTimeout); } while (!IsApplicationExitRequested()); ``` to ```cpp do { ShutdownRequested = m_ShutdownEvent.Wait(WaitTimeout); } while (!ShutdownRequested); ``` That was well-intentioned — tests wanted to start/stop an HTTP server without touching global process state — but the old loop was the only thing that turned `RequestApplicationExit()` into an actual server wake-up. Once it was removed, `RequestApplicationExit(0)` was silently downgraded to "just sets a flag". The `WindowsService::SvcCtrlHandler` stop path was calling exactly that, so SCM stops stopped working. The sponsor-process check path kept working only because it *also* calls `m_Http->RequestExit()` via `ZenServerBase::RequestExit()`. ## Fix - Restore `IsApplicationExitRequested()` as a secondary exit condition in each HTTP server's `OnRun` loop (`httpsys`, `httpasio`, `httpmulti`, `httpnull`, `httpplugin`) alongside the per-server `m_ShutdownEvent` that #751 introduced. Preserves #751's goal — tests can still call `server->RequestExit()` without touching global state — while making `RequestApplicationExit()` wake the server up again, which the rest of the codebase and `SvcCtrlHandler` assume. - Clean up the service control handler in the same pass: also accept `SERVICE_CONTROL_SHUTDOWN`, report `STOP_PENDING` with a 30s `dwWaitHint` (was 0), drop the redundant second `ReportSvcStatus` call, and remove `ghSvcStopEvent` which nothing ever `Wait()`-ed on. - Advertise `SERVICE_ACCEPT_STOP | SERVICE_ACCEPT_SHUTDOWN` while running; drop controls while stop-pending/stopped. - Make `WindowsService` destructor virtual (latent UB given `Run()` was already virtual).
-rw-r--r--CHANGELOG.md1
-rw-r--r--src/zenhttp/servers/httpasio.cpp4
-rw-r--r--src/zenhttp/servers/httpmulti.cpp4
-rw-r--r--src/zenhttp/servers/httpnull.cpp4
-rw-r--r--src/zenhttp/servers/httpplugin.cpp10
-rw-r--r--src/zenhttp/servers/httpsys.cpp2
-rw-r--r--src/zenutil/include/zenutil/windows/windowsservice.h2
-rw-r--r--src/zenutil/windows/windowsservice.cpp39
8 files changed, 31 insertions, 35 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md
index b54cd4f5f..84c0ae8ac 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -40,6 +40,7 @@
- Improvement: S3 client fails fast with a "no credentials available" error when AWS credentials are missing, instead of sending an unsigned request that S3 rejects with a generic 400
- Improvement: IMDS credential provider retries transient connection failures (up to 3 attempts with backoff)
- Improvement: HTTP clients with `RetryCount > 0` also retry on `CURLE_COULDNT_CONNECT`
+- Bugfix: Fix Windows service shutdown signalling so it works as intended again
- Bugfix: `builds download` partial-block fetch decisions now account for build storage host latency
- Bugfix: Transfer rate displays in `builds` commands now smooth correctly
- Bugfix: Structured cache PUT errors with a detail body no longer write the HTTP response twice
diff --git a/src/zenhttp/servers/httpasio.cpp b/src/zenhttp/servers/httpasio.cpp
index a1a775ba3..b624c3a29 100644
--- a/src/zenhttp/servers/httpasio.cpp
+++ b/src/zenhttp/servers/httpasio.cpp
@@ -2618,7 +2618,7 @@ HttpAsioServer::OnRun(bool IsInteractive)
}
ShutdownRequested = m_ShutdownEvent.Wait(WaitTimeout);
- } while (!ShutdownRequested);
+ } while (!ShutdownRequested && !IsApplicationExitRequested());
#else
if (IsInteractive)
{
@@ -2628,7 +2628,7 @@ HttpAsioServer::OnRun(bool IsInteractive)
do
{
ShutdownRequested = m_ShutdownEvent.Wait(WaitTimeout);
- } while (!ShutdownRequested);
+ } while (!ShutdownRequested && !IsApplicationExitRequested());
#endif
}
diff --git a/src/zenhttp/servers/httpmulti.cpp b/src/zenhttp/servers/httpmulti.cpp
index 584e06cbf..196c0c142 100644
--- a/src/zenhttp/servers/httpmulti.cpp
+++ b/src/zenhttp/servers/httpmulti.cpp
@@ -88,7 +88,7 @@ HttpMultiServer::OnRun(bool IsInteractiveSession)
}
ShutdownRequested = m_ShutdownEvent.Wait(WaitTimeout);
- } while (!ShutdownRequested);
+ } while (!ShutdownRequested && !IsApplicationExitRequested());
#else
if (IsInteractiveSession)
{
@@ -98,7 +98,7 @@ HttpMultiServer::OnRun(bool IsInteractiveSession)
do
{
ShutdownRequested = m_ShutdownEvent.Wait(WaitTimeout);
- } while (!ShutdownRequested);
+ } while (!ShutdownRequested && !IsApplicationExitRequested());
#endif
}
diff --git a/src/zenhttp/servers/httpnull.cpp b/src/zenhttp/servers/httpnull.cpp
index 9bb7ef3bc..d698bcb9d 100644
--- a/src/zenhttp/servers/httpnull.cpp
+++ b/src/zenhttp/servers/httpnull.cpp
@@ -63,7 +63,7 @@ HttpNullServer::OnRun(bool IsInteractiveSession)
}
ShutdownRequested = m_ShutdownEvent.Wait(WaitTimeout);
- } while (!ShutdownRequested);
+ } while (!ShutdownRequested && !IsApplicationExitRequested());
#else
if (IsInteractiveSession)
{
@@ -73,7 +73,7 @@ HttpNullServer::OnRun(bool IsInteractiveSession)
do
{
ShutdownRequested = m_ShutdownEvent.Wait(WaitTimeout);
- } while (!ShutdownRequested);
+ } while (!ShutdownRequested && !IsApplicationExitRequested());
#endif
}
diff --git a/src/zenhttp/servers/httpplugin.cpp b/src/zenhttp/servers/httpplugin.cpp
index b0fb020e0..ad7ed259a 100644
--- a/src/zenhttp/servers/httpplugin.cpp
+++ b/src/zenhttp/servers/httpplugin.cpp
@@ -855,6 +855,7 @@ HttpPluginServerImpl::OnRun(bool IsInteractive)
ZEN_CONSOLE("Zen Server running (plugin HTTP). Press ESC or Q to quit");
}
+ bool ShutdownRequested = false;
do
{
if (IsInteractive && _kbhit() != 0)
@@ -868,18 +869,19 @@ HttpPluginServerImpl::OnRun(bool IsInteractive)
}
}
- m_ShutdownEvent.Wait(WaitTimeout);
- } while (!IsApplicationExitRequested());
+ ShutdownRequested = m_ShutdownEvent.Wait(WaitTimeout);
+ } while (!ShutdownRequested && !IsApplicationExitRequested());
# else
if (IsInteractive)
{
ZEN_CONSOLE("Zen Server running (plugin HTTP). Ctrl-C to quit");
}
+ bool ShutdownRequested = false;
do
{
- m_ShutdownEvent.Wait(WaitTimeout);
- } while (!IsApplicationExitRequested());
+ ShutdownRequested = m_ShutdownEvent.Wait(WaitTimeout);
+ } while (!ShutdownRequested && !IsApplicationExitRequested());
# endif
}
diff --git a/src/zenhttp/servers/httpsys.cpp b/src/zenhttp/servers/httpsys.cpp
index 67b1230a0..c1b426bea 100644
--- a/src/zenhttp/servers/httpsys.cpp
+++ b/src/zenhttp/servers/httpsys.cpp
@@ -1727,7 +1727,7 @@ HttpSysServer::OnRun(bool IsInteractive)
ShutdownRequested = m_ShutdownEvent.Wait(WaitTimeout);
UpdateLofreqTimerValue();
- } while (!ShutdownRequested);
+ } while (!ShutdownRequested && !IsApplicationExitRequested());
}
void
diff --git a/src/zenutil/include/zenutil/windows/windowsservice.h b/src/zenutil/include/zenutil/windows/windowsservice.h
index ca0270a36..d7b3347a9 100644
--- a/src/zenutil/include/zenutil/windows/windowsservice.h
+++ b/src/zenutil/include/zenutil/windows/windowsservice.h
@@ -8,7 +8,7 @@ class WindowsService
{
public:
WindowsService();
- ~WindowsService();
+ virtual ~WindowsService();
virtual int Run() = 0;
diff --git a/src/zenutil/windows/windowsservice.cpp b/src/zenutil/windows/windowsservice.cpp
index ebb88b018..383568650 100644
--- a/src/zenutil/windows/windowsservice.cpp
+++ b/src/zenutil/windows/windowsservice.cpp
@@ -16,7 +16,6 @@
SERVICE_STATUS gSvcStatus;
SERVICE_STATUS_HANDLE gSvcStatusHandle;
-HANDLE ghSvcStopEvent = NULL;
void SvcInstall(void);
@@ -205,21 +204,6 @@ WindowsService::SvcMain()
ReportSvcStatus(SERVICE_START_PENDING, NO_ERROR, 3000);
- // Create an event. The control handler function, SvcCtrlHandler,
- // signals this event when it receives the stop control code.
-
- ghSvcStopEvent = CreateEvent(NULL, // default security attributes
- TRUE, // manual reset event
- FALSE, // not signaled
- NULL); // no name
-
- if (ghSvcStopEvent == NULL)
- {
- ReportSvcStatus(SERVICE_STOPPED, GetLastError(), 0);
-
- return 1;
- }
-
int ReturnCode = Run();
return ReturnCode;
}
@@ -549,9 +533,18 @@ ReportSvcStatus(DWORD dwCurrentState, DWORD dwWin32ExitCode, DWORD dwWaitHint)
gSvcStatus.dwWaitHint = dwWaitHint;
if (dwCurrentState == SERVICE_START_PENDING)
+ {
gSvcStatus.dwControlsAccepted = 0;
+ }
+ else if (dwCurrentState == SERVICE_STOP_PENDING || dwCurrentState == SERVICE_STOPPED)
+ {
+ // We are already stopping/stopped - don't accept further control codes.
+ gSvcStatus.dwControlsAccepted = 0;
+ }
else
- gSvcStatus.dwControlsAccepted = SERVICE_ACCEPT_STOP;
+ {
+ gSvcStatus.dwControlsAccepted = SERVICE_ACCEPT_STOP | SERVICE_ACCEPT_SHUTDOWN;
+ }
if ((dwCurrentState == SERVICE_RUNNING) || (dwCurrentState == SERVICE_STOPPED))
gSvcStatus.dwCheckPoint = 0;
@@ -573,14 +566,14 @@ WindowsService::SvcCtrlHandler(DWORD dwCtrl)
switch (dwCtrl)
{
case SERVICE_CONTROL_STOP:
- ReportSvcStatus(SERVICE_STOP_PENDING, NO_ERROR, 0);
+ case SERVICE_CONTROL_SHUTDOWN:
+ // SCM gives us dwWaitHint milliseconds to complete the stop; 30s leaves
+ // plenty of headroom for the HTTP server and service state to tear down.
+ ReportSvcStatus(SERVICE_STOP_PENDING, NO_ERROR, 30000);
- // Signal the service to stop.
-
- SetEvent(ghSvcStopEvent);
+ // The HTTP server run loops poll IsApplicationExitRequested(), so this
+ // flag is how we ask them to wind down.
zen::RequestApplicationExit(0);
-
- ReportSvcStatus(gSvcStatus.dwCurrentState, NO_ERROR, 0);
return;
case SERVICE_CONTROL_INTERROGATE: