services: Track services and processes separately. (v2)
Sebastian Lackner
sebastian at fds-team.de
Wed Apr 20 07:51:30 CDT 2016
Signed-off-by: Sebastian Lackner <sebastian at fds-team.de>
---
Changes in v2:
* Fixed test failure in taskschd on some machines. This was a race-condition introduced
while refactoring the code. (thanks Alexandre).
* Explicitly initialize 'process' in service_start. Should not be necessary, but avoids a
compiler warning on some versions of gcc. (thanks Dmitry)
* Simplify scmdatabase_wait_terminate and do not unset process->db until it is really gone.
The patch can be applied independently from 121540.
programs/services/rpc.c | 37 ++++---
programs/services/services.c | 207 ++++++++++++++++++++++++++-----------------
programs/services/services.h | 4
3 files changed, 151 insertions(+), 97 deletions(-)
diff --git a/programs/services/rpc.c b/programs/services/rpc.c
index babb2b5..3ebf378 100644
--- a/programs/services/rpc.c
+++ b/programs/services/rpc.c
@@ -100,23 +100,22 @@ struct timeout_queue_elem
struct list entry;
FILETIME time;
- void (*func)(struct service_entry*);
- struct service_entry *service_entry;
+ void (*func)(struct process_entry *);
+ struct process_entry *process;
};
-static void run_after_timeout(void (*func)(struct service_entry*), struct service_entry *service, DWORD timeout)
+static void run_after_timeout(void (*func)(struct process_entry *), struct process_entry *process, DWORD timeout)
{
struct timeout_queue_elem *elem = HeapAlloc(GetProcessHeap(), 0, sizeof(struct timeout_queue_elem));
ULARGE_INTEGER time;
if(!elem) {
- func(service);
+ func(process);
return;
}
- InterlockedIncrement(&service->ref_count);
elem->func = func;
- elem->service_entry = service;
+ elem->process = grab_process(process);
GetSystemTimeAsFileTime(&elem->time);
time.u.LowPart = elem->time.dwLowDateTime;
@@ -751,6 +750,7 @@ DWORD __cdecl svcctl_SetServiceStatus(
LPSERVICE_STATUS lpServiceStatus)
{
struct sc_service_handle *service;
+ struct process_entry *process;
DWORD err;
WINE_TRACE("(%p, %p)\n", hServiceStatus, lpServiceStatus);
@@ -768,13 +768,15 @@ DWORD __cdecl svcctl_SetServiceStatus(
service->service_entry->status.dwServiceSpecificExitCode = lpServiceStatus->dwServiceSpecificExitCode;
service->service_entry->status.dwCheckPoint = lpServiceStatus->dwCheckPoint;
service->service_entry->status.dwWaitHint = lpServiceStatus->dwWaitHint;
+ if ((process = service->service_entry->process))
+ {
+ if (lpServiceStatus->dwCurrentState == SERVICE_STOPPED)
+ run_after_timeout(process_terminate, process, service_kill_timeout);
+ else
+ SetEvent(process->status_changed_event);
+ }
service_unlock(service->service_entry);
- if (lpServiceStatus->dwCurrentState == SERVICE_STOPPED)
- run_after_timeout(service_terminate, service->service_entry, service_kill_timeout);
- else if (service->service_entry->process->status_changed_event)
- SetEvent(service->service_entry->process->status_changed_event);
-
return ERROR_SUCCESS;
}
@@ -1173,6 +1175,9 @@ DWORD __cdecl svcctl_ControlService(
process = grab_process(service->service_entry->process);
service_unlock(service->service_entry);
+ if (!process)
+ return ERROR_SERVICE_CANNOT_ACCEPT_CTRL;
+
ret = WaitForSingleObject(process->control_mutex, 30000);
if (ret != WAIT_OBJECT_0)
{
@@ -1930,7 +1935,7 @@ DWORD events_loop(void)
WINE_TRACE("Exceeded maximum wait object count\n");
break;
}
- wait_handles[num_handles] = iter->service_entry->process->process;
+ wait_handles[num_handles] = iter->process->process;
num_handles++;
}
LeaveCriticalSection(&timeout_queue_cs);
@@ -1956,10 +1961,10 @@ DWORD events_loop(void)
(err > WAIT_OBJECT_0 + 1 && idx == err - WAIT_OBJECT_0 - 2))
{
LeaveCriticalSection(&timeout_queue_cs);
- iter->func(iter->service_entry);
+ iter->func(iter->process);
EnterCriticalSection(&timeout_queue_cs);
- release_service(iter->service_entry);
+ release_process(iter->process);
list_remove(&iter->entry);
HeapFree(GetProcessHeap(), 0, iter);
}
@@ -1988,10 +1993,10 @@ DWORD events_loop(void)
LIST_FOR_EACH_ENTRY_SAFE(iter, iter_safe, &timeout_queue, struct timeout_queue_elem, entry)
{
LeaveCriticalSection(&timeout_queue_cs);
- iter->func(iter->service_entry);
+ iter->func(iter->process);
EnterCriticalSection(&timeout_queue_cs);
- release_service(iter->service_entry);
+ release_process(iter->process);
list_remove(&iter->entry);
HeapFree(GetProcessHeap(), 0, iter);
}
diff --git a/programs/services/services.c b/programs/services/services.c
index 234c0ac..f0989ae 100644
--- a/programs/services/services.c
+++ b/programs/services/services.c
@@ -69,15 +69,40 @@ static const WCHAR SZ_DESCRIPTION[] = {'D','e','s','c','r','i','p','t','i'
static const WCHAR SZ_PRESHUTDOWN[] = {'P','r','e','s','h','u','t','d','o','w','n','T','i','m','e','o','u','t',0};
static const WCHAR SZ_WOW64[] = {'W','O','W','6','4',0};
-static DWORD process_create(struct process_entry **entry)
+static DWORD process_create(const WCHAR *name, struct process_entry **entry)
{
+ DWORD err;
+
*entry = HeapAlloc(GetProcessHeap(), HEAP_ZERO_MEMORY, sizeof(**entry));
if (!*entry)
return ERROR_NOT_ENOUGH_SERVER_MEMORY;
(*entry)->ref_count = 1;
- (*entry)->control_pipe = INVALID_HANDLE_VALUE;
+ (*entry)->control_mutex = CreateMutexW(NULL, TRUE, NULL);
+ if (!(*entry)->control_mutex)
+ goto error;
+ (*entry)->overlapped_event = CreateEventW(NULL, TRUE, FALSE, NULL);
+ if (!(*entry)->overlapped_event)
+ goto error;
+ (*entry)->status_changed_event = CreateEventW(NULL, FALSE, FALSE, NULL);
+ if (!(*entry)->status_changed_event)
+ goto error;
+ (*entry)->control_pipe = CreateNamedPipeW(name, PIPE_ACCESS_DUPLEX | FILE_FLAG_OVERLAPPED,
+ PIPE_TYPE_BYTE|PIPE_WAIT, 1, 256, 256, 10000, NULL);
+ if ((*entry)->control_pipe == INVALID_HANDLE_VALUE)
+ goto error;
/* all other fields are zero */
return ERROR_SUCCESS;
+
+error:
+ err = GetLastError();
+ if ((*entry)->control_mutex)
+ CloseHandle((*entry)->control_mutex);
+ if ((*entry)->overlapped_event)
+ CloseHandle((*entry)->overlapped_event);
+ if ((*entry)->status_changed_event)
+ CloseHandle((*entry)->status_changed_event);
+ HeapFree(GetProcessHeap(), 0, *entry);
+ return err;
}
static void free_process_entry(struct process_entry *entry)
@@ -92,8 +117,6 @@ static void free_process_entry(struct process_entry *entry)
DWORD service_create(LPCWSTR name, struct service_entry **entry)
{
- DWORD err;
-
*entry = HeapAlloc(GetProcessHeap(), HEAP_ZERO_MEMORY, sizeof(**entry));
if (!*entry)
return ERROR_NOT_ENOUGH_SERVER_MEMORY;
@@ -103,12 +126,6 @@ DWORD service_create(LPCWSTR name, struct service_entry **entry)
HeapFree(GetProcessHeap(), 0, *entry);
return ERROR_NOT_ENOUGH_SERVER_MEMORY;
}
- if ((err = process_create(&(*entry)->process)) != ERROR_SUCCESS)
- {
- HeapFree(GetProcessHeap(), 0, (*entry)->name);
- HeapFree(GetProcessHeap(), 0, *entry);
- return err;
- }
(*entry)->ref_count = 1;
(*entry)->status.dwCurrentState = SERVICE_STOPPED;
(*entry)->status.dwWin32ExitCode = ERROR_SERVICE_NEVER_STARTED;
@@ -128,7 +145,7 @@ void free_service_entry(struct service_entry *entry)
HeapFree(GetProcessHeap(), 0, entry->description);
HeapFree(GetProcessHeap(), 0, entry->dependOnServices);
HeapFree(GetProcessHeap(), 0, entry->dependOnGroups);
- release_process(entry->process);
+ if (entry->process) release_process(entry->process);
HeapFree(GetProcessHeap(), 0, entry);
}
@@ -268,6 +285,18 @@ cleanup:
return err;
}
+static void scmdatabase_add_process(struct scmdatabase *db, struct process_entry *process)
+{
+ process->db = db;
+ list_add_tail(&db->processes, &process->entry);
+}
+
+static void scmdatabase_remove_process(struct scmdatabase *db, struct process_entry *process)
+{
+ list_remove(&process->entry);
+ process->entry.next = process->entry.prev = NULL;
+}
+
DWORD scmdatabase_add_service(struct scmdatabase *db, struct service_entry *service)
{
int err;
@@ -349,27 +378,22 @@ static void scmdatabase_autostart_services(struct scmdatabase *db)
static void scmdatabase_wait_terminate(struct scmdatabase *db)
{
- struct service_entry *service;
- BOOL run = TRUE;
+ struct list pending = LIST_INIT(pending);
+ void *ptr;
scmdatabase_lock(db);
- while(run)
+ list_move_tail(&pending, &db->processes);
+ while ((ptr = list_head(&pending)))
{
- run = FALSE;
- LIST_FOR_EACH_ENTRY(service, &db->services, struct service_entry, entry)
- {
- struct process_entry *process = service->process;
- if (process->process)
- {
- scmdatabase_unlock(db);
- WaitForSingleObject(process->process, INFINITE);
- scmdatabase_lock(db);
- CloseHandle(process->process);
- process->process = NULL;
- run = TRUE;
- break;
- }
- }
+ struct process_entry *process = grab_process(LIST_ENTRY(ptr, struct process_entry, entry));
+
+ scmdatabase_unlock(db);
+ WaitForSingleObject(process->process, INFINITE);
+ scmdatabase_lock(db);
+
+ list_remove(&process->entry);
+ list_add_tail(&db->processes, &process->entry);
+ release_process(process);
}
scmdatabase_unlock(db);
}
@@ -466,8 +490,15 @@ struct process_entry *grab_process(struct process_entry *process)
void release_process(struct process_entry *process)
{
+ struct scmdatabase *db = process->db;
+
+ scmdatabase_lock(db);
if (InterlockedDecrement(&process->ref_count) == 0)
+ {
+ scmdatabase_remove_process(db, process);
free_process_entry(process);
+ }
+ scmdatabase_unlock(db);
}
void release_service(struct service_entry *service)
@@ -492,6 +523,7 @@ static DWORD scmdatabase_create(struct scmdatabase **db)
return ERROR_NOT_ENOUGH_SERVER_MEMORY;
(*db)->service_start_lock = FALSE;
+ list_init(&(*db)->processes);
list_init(&(*db)->services);
InitializeCriticalSection(&(*db)->cs);
@@ -704,8 +736,9 @@ static DWORD get_service_binary_path(const struct service_entry *service_entry,
return ERROR_SUCCESS;
}
-static DWORD service_start_process(struct service_entry *service_entry)
+static DWORD service_start_process(struct service_entry *service_entry, struct process_entry **new_process)
{
+ struct process_entry *process;
PROCESS_INFORMATION pi;
STARTUPINFOW si;
LPWSTR path = NULL;
@@ -714,9 +747,31 @@ static DWORD service_start_process(struct service_entry *service_entry)
service_lock(service_entry);
+ if ((process = service_entry->process))
+ {
+ if (WaitForSingleObject(process->process, 0) == WAIT_TIMEOUT)
+ {
+ service_unlock(service_entry);
+ return ERROR_SERVICE_ALREADY_RUNNING;
+ }
+ release_process(process);
+ service_entry->process = NULL;
+ }
+
+ service_entry->force_shutdown = FALSE;
+
+ if ((err = process_create(service_get_pipe_name(), &process)))
+ {
+ WINE_ERR("failed to create process object for %s, error = %u\n",
+ wine_dbgstr_w(service_entry->name), err);
+ service_unlock(service_entry);
+ return err;
+ }
+
if ((err = get_service_binary_path(service_entry, &path)))
{
service_unlock(service_entry);
+ free_process_entry(process);
return err;
}
@@ -729,6 +784,8 @@ static DWORD service_start_process(struct service_entry *service_entry)
}
service_entry->status.dwCurrentState = SERVICE_START_PENDING;
+ scmdatabase_add_process(service_entry->db, process);
+ service_entry->process = process;
service_unlock(service_entry);
@@ -736,16 +793,16 @@ static DWORD service_start_process(struct service_entry *service_entry)
HeapFree(GetProcessHeap(),0,path);
if (!r)
{
- service_lock(service_entry);
- service_entry->status.dwCurrentState = SERVICE_STOPPED;
- service_unlock(service_entry);
- return GetLastError();
+ err = GetLastError();
+ service_terminate(service_entry);
+ return err;
}
service_entry->status.dwProcessId = pi.dwProcessId;
- service_entry->process->process = pi.hProcess;
+ process->process = pi.hProcess;
CloseHandle( pi.hThread );
+ *new_process = process;
return ERROR_SUCCESS;
}
@@ -843,56 +900,31 @@ static BOOL process_send_start_message(struct process_entry *process, const WCHA
DWORD service_start(struct service_entry *service, DWORD service_argc, LPCWSTR *service_argv)
{
- struct process_entry *process = service->process;
+ struct process_entry *process = NULL;
DWORD err;
err = scmdatabase_lock_startup(service->db);
if (err != ERROR_SUCCESS)
return err;
- if (WaitForSingleObject(process->process, 0) == WAIT_TIMEOUT)
- {
- scmdatabase_unlock_startup(service->db);
- return ERROR_SERVICE_ALREADY_RUNNING;
- }
-
- CloseHandle(process->control_pipe);
- process->control_mutex = CreateMutexW(NULL, TRUE, NULL);
- service->force_shutdown = FALSE;
-
- if (!process->status_changed_event)
- process->status_changed_event = CreateEventW(NULL, FALSE, FALSE, NULL);
- if (!process->overlapped_event)
- process->overlapped_event = CreateEventW(NULL, TRUE, FALSE, NULL);
-
- process->control_pipe = CreateNamedPipeW(service_get_pipe_name(), PIPE_ACCESS_DUPLEX |
- FILE_FLAG_OVERLAPPED, PIPE_TYPE_BYTE|PIPE_WAIT, 1, 256, 256, 10000, NULL );
- if (process->control_pipe == INVALID_HANDLE_VALUE)
- {
- WINE_ERR("failed to create pipe for %s, error = %d\n",
- wine_dbgstr_w(service->name), GetLastError());
- err = GetLastError();
- }
- else
+ err = service_start_process(service, &process);
+ if (err == ERROR_SUCCESS)
{
- err = service_start_process(service);
- if (err == ERROR_SUCCESS)
- {
- if (!process_send_start_message(process, service->name, service_argv, service_argc))
- err = ERROR_SERVICE_REQUEST_TIMEOUT;
- }
+ if (!process_send_start_message(process, service->name, service_argv, service_argc))
+ err = ERROR_SERVICE_REQUEST_TIMEOUT;
if (err == ERROR_SUCCESS)
err = process_wait_for_startup(process);
if (err == ERROR_SUCCESS)
err = service_is_running(service);
+
+ if (err == ERROR_SUCCESS)
+ ReleaseMutex(process->control_mutex);
+ else
+ service_terminate(service);
}
- if (err == ERROR_SUCCESS)
- ReleaseMutex(process->control_mutex);
- else
- service_terminate(service);
scmdatabase_unlock_startup(service->db);
WINE_TRACE("returning %d\n", err);
@@ -902,24 +934,37 @@ DWORD service_start(struct service_entry *service, DWORD service_argc, LPCWSTR *
void service_terminate(struct service_entry *service)
{
- struct process_entry *process = service->process;
+ struct process_entry *process;
service_lock(service);
- TerminateProcess(process->process, 0);
- CloseHandle(process->process);
- process->process = NULL;
- CloseHandle(process->status_changed_event);
- process->status_changed_event = NULL;
- CloseHandle(process->control_mutex);
- process->control_mutex = NULL;
- CloseHandle(process->control_pipe);
- process->control_pipe = INVALID_HANDLE_VALUE;
-
+ if ((process = service->process))
+ {
+ TerminateProcess(process->process, 0);
+ release_process(process);
+ service->process = NULL;
+ }
service->status.dwProcessId = 0;
service->status.dwCurrentState = SERVICE_STOPPED;
service_unlock(service);
}
+void process_terminate(struct process_entry *process)
+{
+ struct scmdatabase *db = process->db;
+ struct service_entry *service;
+
+ scmdatabase_lock(db);
+ TerminateProcess(process->process, 0);
+ LIST_FOR_EACH_ENTRY(service, &db->services, struct service_entry, entry)
+ {
+ if (service->process != process) continue;
+ service->status.dwProcessId = 0;
+ service->status.dwCurrentState = SERVICE_STOPPED;
+ service->process = NULL;
+ }
+ scmdatabase_unlock(db);
+}
+
static void load_registry_parameters(void)
{
static const WCHAR controlW[] =
diff --git a/programs/services/services.h b/programs/services/services.h
index 3d6c486..ff9d582 100644
--- a/programs/services/services.h
+++ b/programs/services/services.h
@@ -27,12 +27,15 @@ struct scmdatabase
{
HKEY root_key;
LONG service_start_lock;
+ struct list processes;
struct list services;
CRITICAL_SECTION cs;
};
struct process_entry
{
+ struct list entry;
+ struct scmdatabase *db;
LONG ref_count;
HANDLE process;
HANDLE control_mutex;
@@ -91,6 +94,7 @@ void service_terminate(struct service_entry *service);
struct process_entry *grab_process(struct process_entry *process);
void release_process(struct process_entry *process);
BOOL process_send_command(struct process_entry *process, const void *data, DWORD size, DWORD *result);
+void process_terminate(struct process_entry *process);
extern HANDLE g_hStartedEvent;
--
2.7.1
More information about the wine-patches
mailing list