From 9183a171f16aec2c5c2c0d2746d8a6e3a2d3e134 Mon Sep 17 00:00:00 2001 From: Francois Gouget Date: Mon, 29 Aug 2011 19:52:45 +0200 Subject: [PATCH] services: Cleanup when a service fails to start so it is still fully considered to be stopped. --- dlls/advapi32/tests/service.c | 25 +++++++++++------------- programs/services/services.c | 44 ++++++++++++++++++++++++++++++------------- 2 files changed, 42 insertions(+), 27 deletions(-) diff --git a/dlls/advapi32/tests/service.c b/dlls/advapi32/tests/service.c index 9b6e3136db8..044394a9786 100644 --- a/dlls/advapi32/tests/service.c +++ b/dlls/advapi32/tests/service.c @@ -2065,7 +2065,7 @@ cleanup: CloseServiceHandle(scm_handle); } -static DWORD try_start_stop(SC_HANDLE svc_handle, const char* name, int todo, DWORD is_nt4) +static DWORD try_start_stop(SC_HANDLE svc_handle, const char* name, DWORD is_nt4) { BOOL ret; DWORD le1, le2; @@ -2082,24 +2082,21 @@ static DWORD try_start_stop(SC_HANDLE svc_handle, const char* name, int todo, DW ret = pQueryServiceStatusEx(svc_handle, SC_STATUS_PROCESS_INFO, (BYTE*)&statusproc, sizeof(statusproc), &needed); ok(ret, "%s: QueryServiceStatusEx() failed le=%u\n", name, GetLastError()); - todo_wine ok(statusproc.dwCurrentState == SERVICE_STOPPED, "%s: should be stopped state=%x\n", name, statusproc.dwCurrentState); - todo_wine ok(statusproc.dwProcessId == 0, "%s: ProcessId should be 0 instead of %x\n", name, statusproc.dwProcessId); + ok(statusproc.dwCurrentState == SERVICE_STOPPED, "%s: should be stopped state=%x\n", name, statusproc.dwCurrentState); + ok(statusproc.dwProcessId == 0, "%s: ProcessId should be 0 instead of %x\n", name, statusproc.dwProcessId); } ret = StartServiceA(svc_handle, 0, NULL); le2 = GetLastError(); ok(!ret, "%s: StartServiceA() should have failed\n", name); - if (todo) - todo_wine ok(le2 == le1, "%s: the second try should yield the same error: %u != %u\n", name, le1, le2); - else - ok(le2 == le1, "%s: the second try should yield the same error: %u != %u\n", name, le1, le2); + ok(le2 == le1, "%s: the second try should yield the same error: %u != %u\n", name, le1, le2); status.dwCurrentState = 0xdeadbeef; ret = ControlService(svc_handle, SERVICE_CONTROL_STOP, &status); le2 = GetLastError(); ok(!ret, "%s: ControlService() should have failed\n", name); todo_wine ok(le2 == ERROR_SERVICE_NOT_ACTIVE, "%s: %d != ERROR_SERVICE_NOT_ACTIVE\n", name, le2); - todo_wine ok(status.dwCurrentState == SERVICE_STOPPED || + ok(status.dwCurrentState == SERVICE_STOPPED || broken(is_nt4), /* NT4 returns a random value */ "%s: should be stopped state=%x\n", name, status.dwCurrentState); @@ -2153,14 +2150,14 @@ static void test_start_stop(void) ok(FALSE, "Could not create the service: %d\n", GetLastError()); goto cleanup; } - le = try_start_stop(svc_handle, displayname, 1, is_nt4); + le = try_start_stop(svc_handle, displayname, is_nt4); todo_wine ok(le == ERROR_SERVICE_DISABLED, "%d != ERROR_SERVICE_DISABLED\n", le); /* Then one with a bad path */ displayname = "Winetest Bad Path"; ret = ChangeServiceConfigA(svc_handle, SERVICE_NO_CHANGE, SERVICE_DEMAND_START, SERVICE_NO_CHANGE, "c:\\no_such_file.exe", NULL, NULL, NULL, NULL, NULL, displayname); ok(ret, "ChangeServiceConfig() failed le=%u\n", GetLastError()); - try_start_stop(svc_handle, displayname, 0, is_nt4); + try_start_stop(svc_handle, displayname, is_nt4); if (is_nt4) { @@ -2175,8 +2172,8 @@ static void test_start_stop(void) displayname = "Winetest Exit Service"; ret = ChangeServiceConfigA(svc_handle, SERVICE_NO_CHANGE, SERVICE_NO_CHANGE, SERVICE_NO_CHANGE, cmd, NULL, NULL, NULL, NULL, NULL, displayname); ok(ret, "ChangeServiceConfig() failed le=%u\n", GetLastError()); - le = try_start_stop(svc_handle, displayname, 0, is_nt4); - todo_wine ok(le == ERROR_SERVICE_REQUEST_TIMEOUT, "%d != ERROR_SERVICE_REQUEST_TIMEOUT\n", le); + le = try_start_stop(svc_handle, displayname, is_nt4); + ok(le == ERROR_SERVICE_REQUEST_TIMEOUT, "%d != ERROR_SERVICE_REQUEST_TIMEOUT\n", le); /* And finally with a service that plays dead, forcing a timeout. * This time we will put no quotes. That should work too, even if there are @@ -2187,8 +2184,8 @@ static void test_start_stop(void) ret = ChangeServiceConfigA(svc_handle, SERVICE_NO_CHANGE, SERVICE_NO_CHANGE, SERVICE_NO_CHANGE, cmd, NULL, NULL, NULL, NULL, NULL, displayname); ok(ret, "ChangeServiceConfig() failed le=%u\n", GetLastError()); - le = try_start_stop(svc_handle, displayname, 0, is_nt4); - todo_wine ok(le == ERROR_SERVICE_REQUEST_TIMEOUT, "%d != ERROR_SERVICE_REQUEST_TIMEOUT\n", le); + le = try_start_stop(svc_handle, displayname, is_nt4); + ok(le == ERROR_SERVICE_REQUEST_TIMEOUT, "%d != ERROR_SERVICE_REQUEST_TIMEOUT\n", le); cleanup: if (svc_handle) diff --git a/programs/services/services.c b/programs/services/services.c index 35edbbaa44c..742f17d24cb 100644 --- a/programs/services/services.c +++ b/programs/services/services.c @@ -772,25 +772,43 @@ DWORD service_start(struct service_entry *service, DWORD service_argc, LPCWSTR * { WINE_ERR("failed to create pipe for %s, error = %d\n", wine_dbgstr_w(service->name), GetLastError()); - scmdatabase_unlock_startup(service->db); - return GetLastError(); + err = GetLastError(); } + else + { + err = service_start_process(service, &process_handle); + if (err == ERROR_SUCCESS) + { + if (!service_send_start_message(service, process_handle, service_argv, service_argc)) + err = ERROR_SERVICE_REQUEST_TIMEOUT; + } - err = service_start_process(service, &process_handle); + if (err == ERROR_SUCCESS) + err = service_wait_for_startup(service, process_handle); - if (err == ERROR_SUCCESS) - { - if (!service_send_start_message(service, process_handle, service_argv, service_argc)) - err = ERROR_SERVICE_REQUEST_TIMEOUT; + if (process_handle) + CloseHandle(process_handle); } if (err == ERROR_SUCCESS) - err = service_wait_for_startup(service, process_handle); - - if (process_handle) - CloseHandle(process_handle); - - ReleaseMutex(service->control_mutex); + ReleaseMutex(service->control_mutex); + else + { + CloseHandle(service->overlapped_event); + service->overlapped_event = NULL; + CloseHandle(service->status_changed_event); + service->status_changed_event = NULL; + CloseHandle(service->control_mutex); + service->control_mutex = NULL; + if (service->control_pipe != INVALID_HANDLE_VALUE) + CloseHandle(service->control_pipe); + service->control_pipe = INVALID_HANDLE_VALUE; + + service->status.dwProcessId = 0; + service_lock_exclusive(service); + service->status.dwCurrentState = SERVICE_STOPPED; + service_unlock(service); + } scmdatabase_unlock_startup(service->db); WINE_TRACE("returning %d\n", err); -- 2.11.4.GIT