diff --git a/src/main_windows.cpp b/src/main_windows.cpp index bee821292d..45759c0841 100644 --- a/src/main_windows.cpp +++ b/src/main_windows.cpp @@ -156,6 +156,7 @@ int main_windows(int argc, char** argv) { SERVICE_STATUS OvmsWindowsServiceManager::serviceStatus = {0}; std::unique_ptr OvmsWindowsServiceManager::statusHandle = std::make_unique(); std::unique_ptr OvmsWindowsServiceManager::serviceStopEvent = std::make_unique(); +std::atomic OvmsWindowsServiceManager::serviceStopRequested{false}; LPSTR OvmsWindowsServiceManager::serviceName = _T("ovms"); LPSTR OvmsWindowsServiceManager::serviceDisplayName = _T("OpenVino Model Server"); LPSTR OvmsWindowsServiceManager::serviceDesc = _T("Hosts models and makes them accessible to software components over standard network protocols."); @@ -184,6 +185,8 @@ struct WinHandleDeleter { void WINAPI OvmsWindowsServiceManager::serviceMain(DWORD argc, LPTSTR* argv) { DEBUG_LOG("ServiceMain: Entry"); + OvmsWindowsServiceManager::serviceStopRequested.store(false, std::memory_order_release); + statusHandle->handle = RegisterServiceCtrlHandler(OvmsWindowsServiceManager::serviceName, OvmsWindowsServiceManager::serviceCtrlHandler); if (this->statusHandle->handle == NULL || this->statusHandle->handle == INVALID_HANDLE_VALUE) { DEBUG_LOG("ServiceMain: RegisterserviceCtrlHandler returned error"); @@ -224,29 +227,68 @@ void WINAPI OvmsWindowsServiceManager::serviceMain(DWORD argc, LPTSTR* argv) { OvmsWindowsServiceManager::instance().parsedParameters = std::get>(paramsOrExit); } + // Create the stop event before starting the worker. The worker checks this + // handle immediately, so creating the thread first introduces a startup race + // where it can observe INVALID_HANDLE_VALUE. + serviceStopEvent->handle = CreateEvent(NULL, TRUE, FALSE, NULL); + if (serviceStopEvent->handle == NULL || serviceStopEvent->handle == INVALID_HANDLE_VALUE) { + const DWORD createEventError = GetLastError(); + DEBUG_LOG("ServiceMain: CreateEvent(serviceStopEvent) returned error"); + serviceReportEvent(std::string("CreateEvent"), createEventError); + SetLastError(createEventError); + this->setServiceStopStatusWithError(); + return; + } + std::unique_ptr mainThread(CreateThread(NULL, 0, OvmsWindowsServiceManager::serviceWorkerThread, &OvmsWindowsServiceManager::instance().parsedParameters, 0, NULL)); if (mainThread.get() == NULL || mainThread.get() == INVALID_HANDLE_VALUE) { // Handle error + const DWORD createThreadError = GetLastError(); DEBUG_LOG("ServiceMain: mainThread == NULL || mainThread == INVALID_HANDLE_VALUE"); - serviceReportEvent("CreateThread"); + serviceReportEvent(std::string("CreateThread"), createThreadError); + SetLastError(createThreadError); + this->setServiceStopStatusWithError(); return; } - // Create stop event to wait on later. - serviceStopEvent->handle = CreateEvent(NULL, TRUE, FALSE, NULL); - if (serviceStopEvent->handle == NULL || serviceStopEvent->handle == INVALID_HANDLE_VALUE) { - DEBUG_LOG("ServiceMain: CreateEvent(serviceStopEvent) returned error"); - serviceReportEvent("CreateEvent"); + DEBUG_LOG("ServiceMain: Waiting for Worker Thread to complete"); + + const DWORD workerWaitResult = WaitForSingleObject(mainThread.get(), INFINITE); + if (workerWaitResult == WAIT_FAILED) { + const DWORD waitError = GetLastError(); + DEBUG_LOG("ServiceMain: WaitForSingleObject(mainThread) failed"); + serviceReportEvent(std::string("WaitForSingleObject"), waitError); + SetLastError(waitError); + this->setServiceStopStatusWithError(); + return; + } + if (workerWaitResult != WAIT_OBJECT_0) { + const DWORD waitError = ERROR_INVALID_FUNCTION; + DEBUG_LOG("ServiceMain: WaitForSingleObject(mainThread) returned unexpected result"); + serviceReportEvent(std::string("WaitForSingleObject"), waitError); + SetLastError(waitError); this->setServiceStopStatusWithError(); return; } - DEBUG_LOG("ServiceMain: Waiting for Worker Thread to complete"); - - WaitForSingleObject(mainThread.get(), INFINITE); DEBUG_LOG("ServiceMain: Worker Thread Stop Event signaled after we leave the WaitForSingle call"); - this->setServiceStopStatusWithSuccess(); + DWORD workerExitCode = ERROR_SUCCESS; + if (GetExitCodeThread(mainThread.get(), &workerExitCode) == FALSE) { + const DWORD exitCodeError = GetLastError(); + DEBUG_LOG("ServiceMain: GetExitCodeThread failed"); + serviceReportEvent(std::string("GetExitCodeThread"), exitCodeError); + SetLastError(exitCodeError); + this->setServiceStopStatusWithError(); + return; + } + + if (workerExitCode == ERROR_SUCCESS) { + this->setServiceStopStatusWithSuccess(); + } else { + DEBUG_LOG("ServiceMain: Worker thread exited with an error"); + this->setServiceStopStatusWithExitCode(static_cast(workerExitCode)); + } DEBUG_LOG("ServiceMain: Exit"); return; @@ -389,17 +431,26 @@ struct WinESHandleDeleter { }; void OvmsWindowsServiceManager::serviceReportEvent(const std::string& szFunction) { - serviceReportEvent(const_cast(szFunction.c_str())); + const DWORD errorCode = GetLastError(); + serviceReportEvent(szFunction, errorCode); } -void OvmsWindowsServiceManager::serviceReportEvent(LPSTR szFunction) { +void OvmsWindowsServiceManager::serviceReportEvent(LPCSTR szFunction) { + const DWORD errorCode = GetLastError(); + serviceReportEvent(szFunction, errorCode); +} + +void OvmsWindowsServiceManager::serviceReportEvent(const std::string& szFunction, DWORD errorCode) { + serviceReportEvent(szFunction.c_str(), errorCode); +} + +void OvmsWindowsServiceManager::serviceReportEvent(LPCSTR szFunction, DWORD errorCode) { LPCTSTR lpszStrings[2]; TCHAR Buffer[200]; std::unique_ptr hEventSource(RegisterEventSource(NULL, OvmsWindowsServiceManager::serviceName)); if (hEventSource.get() != NULL) { - DWORD errcode = GetLastError(); - std::string message = std::system_category().message(errcode); - StringCchPrintf(Buffer, 200, TEXT("%s failed with %lu error: %s"), szFunction, errcode, message.c_str()); + std::string message = std::system_category().message(errorCode); + StringCchPrintf(Buffer, 200, TEXT("%s failed with %lu error: %s"), szFunction, errorCode, message.c_str()); lpszStrings[0] = OvmsWindowsServiceManager::serviceName; lpszStrings[1] = Buffer; ReportEvent(hEventSource.get(), // event log handle @@ -413,8 +464,9 @@ void OvmsWindowsServiceManager::serviceReportEvent(LPSTR szFunction) { NULL); // no binary data } else { + const DWORD registerError = GetLastError(); DEBUG_LOG("RegisterEventSource failed"); - DEBUG_LOG(std::system_category().message(GetLastError())); + DEBUG_LOG(std::system_category().message(registerError)); } } @@ -486,8 +538,19 @@ void WINAPI OvmsWindowsServiceManager::serviceCtrlHandler(DWORD CtrlCode) { break; setServiceStopStatusPending(); + // Keep a lock-free fallback so a transient SetEvent failure cannot + // leave the worker running indefinitely in SERVICE_STOP_PENDING. + serviceStopRequested.store(true, std::memory_order_release); // Signal the worker thread to start shutting down - SetEvent(serviceStopEvent->handle); + if (serviceStopEvent->handle == NULL || serviceStopEvent->handle == INVALID_HANDLE_VALUE) { + DEBUG_LOG("serviceCtrlHandler: serviceStopEvent is invalid"); + serviceReportEvent(std::string("SetEvent"), ERROR_INVALID_HANDLE); + break; + } + if (SetEvent(serviceStopEvent->handle) == FALSE) { + const DWORD eventError = GetLastError(); + serviceReportEvent(std::string("SetEvent"), eventError); + } break; // Currently not supported controls case SERVICE_CONTROL_INTERROGATE: @@ -509,15 +572,32 @@ DWORD WINAPI OvmsWindowsServiceManager::serviceWorkerThread(LPVOID lpParam) { ovmsService->error = 0; ovmsService->started = false; ovmsService->setup = false; + DWORD supervisorError = ERROR_SUCCESS; + + // Start OVMS and check for stop. The finite wait keeps the supervisor + // responsive while preventing a zero-timeout polling loop from consuming a + // complete logical CPU when the server is idle. + constexpr DWORD serviceWorkerPollIntervalMs = 1000; + while (true) { + if (serviceStopRequested.load(std::memory_order_acquire)) { + break; + } + + if (serviceStopEvent->handle == NULL || serviceStopEvent->handle == INVALID_HANDLE_VALUE) { + supervisorError = ERROR_INVALID_HANDLE; + serviceReportEvent(std::string("WaitForSingleObject"), supervisorError); + break; + } - // Start OVMS and check for stop - while (WaitForSingleObject(serviceStopEvent->handle, 0) != WAIT_OBJECT_0) { // Already started if (!ovmsService->setup) { std::pair* params = (std::pair*)lpParam; DEBUG_LOG("serviceWorkerThread: Starting ovms from parameters."); ovmsService->SetUp(params); } + if (serviceStopRequested.load(std::memory_order_acquire)) { + break; + } // Check thread not exited if (!ovmsService->isRunning()) { DEBUG_LOG("serviceWorkerThread: Server thread is not running.") @@ -529,6 +609,24 @@ DWORD WINAPI OvmsWindowsServiceManager::serviceWorkerThread(LPVOID lpParam) { OvmsWindowsServiceManager::setServiceRunningStatus(); ovmsService->started = true; } + + const DWORD waitResult = WaitForSingleObject(serviceStopEvent->handle, serviceWorkerPollIntervalMs); + if (waitResult == WAIT_OBJECT_0) { + break; + } + if (waitResult == WAIT_TIMEOUT) { + continue; + } + + if (waitResult == WAIT_FAILED) { + const DWORD waitError = GetLastError(); + supervisorError = waitError; + serviceReportEvent(std::string("WaitForSingleObject"), supervisorError); + } else { + supervisorError = ERROR_INVALID_FUNCTION; + serviceReportEvent(std::string("WaitForSingleObject"), supervisorError); + } + break; } if (ovmsService->started || ovmsService->setup) { @@ -538,6 +636,12 @@ DWORD WINAPI OvmsWindowsServiceManager::serviceWorkerThread(LPVOID lpParam) { DEBUG_LOG("serviceWorkerThread: Ovms service could not be started."); } + if (supervisorError != ERROR_SUCCESS) { + std::string message = "Windows service supervision failed; Win32 error: " + std::to_string(supervisorError); + serviceReportEventWithExitCode("serviceWorkerThread", message, OVMS_EX_FAILURE); + return OVMS_EX_FAILURE; + } + if (ovmsService->error) { DEBUG_LOG("serviceWorkerThread: Ovms start returned error."); DEBUG_LOG(ovmsService->error); diff --git a/src/main_windows.hpp b/src/main_windows.hpp index 3d87a83094..932b12b89d 100644 --- a/src/main_windows.hpp +++ b/src/main_windows.hpp @@ -17,6 +17,7 @@ #define SRC_MAIN_WINDOWS_HPP_ #endif // SRC_MAIN_WINDOWS_HPP_ +#include #include #include #include @@ -85,8 +86,10 @@ class OvmsWindowsServiceManager { static OvmsWindowsServiceManager& instance(); static std::string getCurrentTimeString(); static void logParameters(DWORD argc, LPTSTR* argv, const std::string& logText); - static void serviceReportEvent(LPSTR szFunction); + static void serviceReportEvent(LPCSTR szFunction); static void serviceReportEvent(const std::string& szFunction); + static void serviceReportEvent(LPCSTR szFunction, DWORD errorCode); + static void serviceReportEvent(const std::string& szFunction, DWORD errorCode); static void serviceReportEventWithExitCode(const std::string& szFunction, const std::string& message, const int& exitCode); static void serviceReportEventWithExitCode(LPSTR szFunction, const std::string& message, const int& exitCode); static void serviceReportEventSuccess(const std::string& szFunction, const std::string& message); @@ -106,6 +109,7 @@ class OvmsWindowsServiceManager { static SERVICE_STATUS serviceStatus; static std::unique_ptr statusHandle; static std::unique_ptr serviceStopEvent; + static std::atomic serviceStopRequested; // Methods static void WINAPI serviceCtrlHandler(DWORD);