diff options
| author | Stefan Boberg <[email protected]> | 2026-04-21 18:28:11 +0200 |
|---|---|---|
| committer | GitHub Enterprise <[email protected]> | 2026-04-21 18:28:11 +0200 |
| commit | 7136132c4c3ae52becab525bc4ce30f3f36126a9 (patch) | |
| tree | cc6f5079abd0e0178cb7f3ae4a871d14e16a1682 | |
| parent | 5.8.6-pre0 (diff) | |
| download | archived-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.md | 1 | ||||
| -rw-r--r-- | src/zenhttp/servers/httpasio.cpp | 4 | ||||
| -rw-r--r-- | src/zenhttp/servers/httpmulti.cpp | 4 | ||||
| -rw-r--r-- | src/zenhttp/servers/httpnull.cpp | 4 | ||||
| -rw-r--r-- | src/zenhttp/servers/httpplugin.cpp | 10 | ||||
| -rw-r--r-- | src/zenhttp/servers/httpsys.cpp | 2 | ||||
| -rw-r--r-- | src/zenutil/include/zenutil/windows/windowsservice.h | 2 | ||||
| -rw-r--r-- | src/zenutil/windows/windowsservice.cpp | 39 |
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: |