Francois Gouget : services: Cleanup when a service fails to start so it is still fully considered to be stopped .
Alexandre Julliard
julliard at winehq.org
Tue Aug 30 12:48:34 CDT 2011
Module: wine
Branch: master
Commit: 9183a171f16aec2c5c2c0d2746d8a6e3a2d3e134
URL: http://source.winehq.org/git/wine.git/?a=commit;h=9183a171f16aec2c5c2c0d2746d8a6e3a2d3e134
Author: Francois Gouget <fgouget at codeweavers.com>
Date: Mon Aug 29 19:52:45 2011 +0200
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 9b6e313..044394a 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 35edbba..742f17d 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);
More information about the wine-cvs
mailing list