services: Defer service delete until all handles are closed.

dlls/advapi32/service.c

@@ -1087,6 +1087,8 @@ BOOL WINAPI DeleteService( SC_HANDLE hService )
 {
     DWORD err;
 
+    TRACE("%p\n", hService);
+
     __TRY
     {
         err = svcctl_DeleteService(hService);

dlls/advapi32/tests/service.c

@@ -187,7 +187,7 @@ static void test_open_svc(void)
 
 static void test_create_delete_svc(void)
 {
-    SC_HANDLE scm_handle, svc_handle1;
+    SC_HANDLE scm_handle, svc_handle1, svc_handle2;
     CHAR username[UNLEN + 1], domain[MAX_PATH];
     DWORD user_size = UNLEN + 1;
     CHAR account[UNLEN + 3];
@@ -412,6 +412,11 @@ static void test_create_delete_svc(void)
     ret = DeleteService(svc_handle1);
     ok(ret, "Expected success, got error %u\n", GetLastError());
 
+    /* Service is marked for delete, but handle is still open. Try to open service again. */
+    svc_handle2 = OpenServiceA(scm_handle, servicename, GENERIC_READ);
+    ok(svc_handle2 != NULL, "got %p, error %u\n", svc_handle2, GetLastError());
+    CloseServiceHandle(svc_handle2);
+
     CloseServiceHandle(svc_handle1);
     CloseServiceHandle(scm_handle);
 
@@ -2341,19 +2346,9 @@ static void test_refcount(void)
     svc_handle5 = CreateServiceA(scm_handle, servicename, NULL, GENERIC_ALL,
                                  SERVICE_WIN32_OWN_PROCESS | SERVICE_INTERACTIVE_PROCESS,
                                  SERVICE_DISABLED, 0, pathname, NULL, NULL, NULL, NULL, NULL);
-    todo_wine
-    {
     ok(!svc_handle5, "Expected failure\n");
     ok(GetLastError() == ERROR_SERVICE_MARKED_FOR_DELETE,
        "Expected ERROR_SERVICE_MARKED_FOR_DELETE, got %d\n", GetLastError());
-    }
-
-    /* FIXME: Remove this when Wine is fixed */
-    if (svc_handle5)
-    {
-        DeleteService(svc_handle5);
-        CloseServiceHandle(svc_handle5);
-    }
 
     /* Close all the handles to the service and try again */
     ret = CloseServiceHandle(svc_handle4);

programs/services/rpc.c

@@ -481,8 +481,8 @@ DWORD __cdecl svcctl_CreateServiceW(
     DWORD dwPasswordSize,
     SC_RPC_HANDLE *phService)
 {
+    struct service_entry *entry, *found;
     struct sc_manager_handle *manager;
-    struct service_entry *entry;
     DWORD err;
 
     WINE_TRACE("(%s, %s, 0x%x, %s)\n", wine_dbgstr_w(lpServiceName), wine_dbgstr_w(lpDisplayName), dwDesiredAccess, wine_dbgstr_w(lpBinaryPathName));
@@ -533,11 +533,14 @@ DWORD __cdecl svcctl_CreateServiceW(
 
     scmdatabase_lock_exclusive(manager->db);
 
-    if (scmdatabase_find_service(manager->db, lpServiceName))
+    if ((found = scmdatabase_find_service(manager->db, lpServiceName)))
     {
+        service_lock_exclusive(found);
+        err = is_marked_for_delete(found) ? ERROR_SERVICE_MARKED_FOR_DELETE : ERROR_SERVICE_EXISTS;
+        service_unlock(found);
         scmdatabase_unlock(manager->db);
         free_service_entry(entry);
-        return ERROR_SERVICE_EXISTS;
+        return err;
     }
 
     if (scmdatabase_find_service_by_displayname(manager->db, get_display_name(entry)))
@@ -568,16 +571,14 @@ DWORD __cdecl svcctl_DeleteService(
     if ((err = validate_service_handle(hService, DELETE, &service)) != ERROR_SUCCESS)
         return err;
 
-    scmdatabase_lock_exclusive(service->service_entry->db);
     service_lock_exclusive(service->service_entry);
 
     if (!is_marked_for_delete(service->service_entry))
-        err = scmdatabase_remove_service(service->service_entry->db, service->service_entry);
+        err = mark_for_delete(service->service_entry);
     else
         err = ERROR_SERVICE_MARKED_FOR_DELETE;
 
     service_unlock(service->service_entry);
-    scmdatabase_unlock(service->service_entry->db);
 
     return err;
 }

programs/services/services.c

@@ -245,7 +245,7 @@ DWORD scmdatabase_add_service(struct scmdatabase *db, struct service_entry *serv
     return ERROR_SUCCESS;
 }
 
-DWORD scmdatabase_remove_service(struct scmdatabase *db, struct service_entry *service)
+static DWORD scmdatabase_remove_service(struct scmdatabase *db, struct service_entry *service)
 {
     int err;
 
@@ -422,7 +422,14 @@ struct service_entry *scmdatabase_find_service_by_displayname(struct scmdatabase
 void release_service(struct service_entry *service)
 {
     if (InterlockedDecrement(&service->ref_count) == 0 && is_marked_for_delete(service))
+    {
+        scmdatabase_lock_exclusive(service->db);
+        service_lock_exclusive(service);
+        scmdatabase_remove_service(service->db, service);
+        service_unlock(service);
+        scmdatabase_unlock(service->db);
         free_service_entry(service);
+    }
 }
 
 static DWORD scmdatabase_create(struct scmdatabase **db)

programs/services/services.h

@@ -48,6 +48,7 @@ struct service_entry
     HANDLE control_pipe;
     HANDLE overlapped_event;
     HANDLE status_changed_event;
+    BOOL marked_for_delete;
 };
 
 extern struct scmdatabase *active_database;
@@ -57,7 +58,6 @@ extern struct scmdatabase *active_database;
 struct service_entry *scmdatabase_find_service(struct scmdatabase *db, LPCWSTR name);
 struct service_entry *scmdatabase_find_service_by_displayname(struct scmdatabase *db, LPCWSTR name);
 DWORD scmdatabase_add_service(struct scmdatabase *db, struct service_entry *entry);
-DWORD scmdatabase_remove_service(struct scmdatabase *db, struct service_entry *entry);
 
 DWORD scmdatabase_lock_startup(struct scmdatabase *db);
 void scmdatabase_unlock_startup(struct scmdatabase *db);
@@ -106,7 +106,13 @@ static inline LPCWSTR get_display_name(struct service_entry *service)
 
 static inline BOOL is_marked_for_delete(struct service_entry *service)
 {
-    return service->entry.next == NULL;
+    return service->marked_for_delete;
+}
+
+static inline DWORD mark_for_delete(struct service_entry *service)
+{
+    service->marked_for_delete = TRUE;
+    return ERROR_SUCCESS;
 }
 
 #endif /*WINE_PROGRAMS_UTILS_H_*/