Re: [advapi32] Create a standard Wine list for the service handles

[Paul Vriens]

Robert Shearman wrote:

Paul Vriens wrote:

Hi,

I've attached a possible solution for checking the validity of the service

handle before using it.

Can somebody please comment on this? Do I need to protect the list with a
CriticalSection?

Yes.

Is this overkill?

Yes.

This should eventually be done via RPC, which has its own separate mechanism for making sure handles are valid.

Ok, so until we have that mechanism the attached should work?

Btw, are there many places in the code where we should use the RPC approach?

Cheers,

Paul.


diff --git a/dlls/advapi32/service.c b/dlls/advapi32/service.c
index 731d668..9e40694 100644
--- a/dlls/advapi32/service.c
+++ b/dlls/advapi32/service.c
@@ -87,7 +87,18 @@ static CRITICAL_SECTION_DEBUG service_cs_debug =
 };
 static CRITICAL_SECTION service_cs = { &service_cs_debug, -1, 0, 0, 0, 0 };
 
+
 static struct list service_list = LIST_INIT(service_list);
+static CRITICAL_SECTION handle_cs;
+static CRITICAL_SECTION_DEBUG handle_cs_debug =
+{
+    0, 0, &handle_cs,
+    { &handle_cs_debug.ProcessLocksList, 
+      &handle_cs_debug.ProcessLocksList },
+      0, 0, { (DWORD_PTR)(__FILE__ ": handle_cs") }
+};
+static CRITICAL_SECTION handle_cs = { &handle_cs_debug, -1, 0, 0, 0, 0 };
+
 
 extern HANDLE __wine_make_process_system(void);
 
@@ -104,6 +115,8 @@ typedef VOID (*sc_handle_destructor)(struct sc_handle *);
 
 struct sc_handle
 {
+    struct list entry;
+    SC_HANDLE handle;
     SC_HANDLE_TYPE htype;
     DWORD ref_count;
     sc_handle_destructor destroy;
@@ -125,41 +138,77 @@ struct sc_service       /* service handle */
     WCHAR  name[1];
 };
 
+static struct list sc_handles = LIST_INIT(sc_handles);
+
 static void *sc_handle_alloc(SC_HANDLE_TYPE htype, DWORD size,
                              sc_handle_destructor destroy)
 {
     struct sc_handle *hdr;
 
+    EnterCriticalSection( &handle_cs );
     hdr = HeapAlloc( GetProcessHeap(), HEAP_ZERO_MEMORY, size );
     if (hdr)
     {
+        hdr->handle = (SC_HANDLE)hdr;
         hdr->htype = htype;
         hdr->ref_count = 1;
         hdr->destroy = destroy;
+        list_add_head(&sc_handles, &hdr->entry);
     }
+    LeaveCriticalSection( &handle_cs );
+
     TRACE("sc_handle type=%d -> %p\n", htype, hdr);
     return hdr;
 }
 
 static void *sc_handle_get_handle_data(SC_HANDLE handle, DWORD htype)
 {
-    struct sc_handle *hdr = (struct sc_handle *) handle;
+    struct sc_handle *hdr = NULL, *cursor;
 
-    if (!hdr)
+    if (!handle)
         return NULL;
-    if (hdr->htype != htype)
+
+    EnterCriticalSection( &handle_cs );
+    LIST_FOR_EACH_ENTRY(cursor, &sc_handles, struct sc_handle, entry)
+    {
+        if (cursor->handle == handle)
+        {
+            hdr = cursor;
+            break;
+        }
+    }
+    LeaveCriticalSection( &handle_cs );
+
+    if (!hdr || (hdr->htype != htype))
         return NULL;
+
     return hdr;
 }
 
 static void sc_handle_free(struct sc_handle* hdr)
 {
+    struct sc_handle *cursor;
+
     if (!hdr)
         return;
-    if (--hdr->ref_count)
-        return;
-    hdr->destroy(hdr);
-    HeapFree(GetProcessHeap(), 0, hdr);
+
+    EnterCriticalSection( &handle_cs );
+    LIST_FOR_EACH_ENTRY(cursor, &sc_handles, struct sc_handle, entry)
+    {
+        if (cursor->handle == (SC_HANDLE)hdr)
+        {
+            if (--hdr->ref_count)
+            {
+                LeaveCriticalSection( &handle_cs );
+                return;
+            }
+            hdr->destroy(hdr);
+            list_remove(&hdr->entry);
+            HeapFree(GetProcessHeap(), 0, hdr);
+            break;
+        }
+    }
+    LeaveCriticalSection( &handle_cs );
 }
 
 static void sc_handle_destroy_manager(struct sc_handle *handle)