RFC: ntdll critical section debug

[Robert Reif]

I have been running into deadlock problems in the direct sound
dll and have been playing with the debug part of critical
sections.  I added a lock name to Spare[1] to give the critical
sections names that will show up in the error messages just
like wine's ntdll does.  After looking at the ntdll code, it
looks like this causes a leak because ntdll statically initializes
it's locks and doesn't delete the debug part when it finds
Spare[1] used.

I also tried adding a linked list of critical section debug structs
just like real windows does.  Because ntdll statically initializes
it's locks, they don't get added to the list.  I manually stuff
the critical section list lock into the list so it shows up and
user locks show up but not the other internal ntdll locks.

Is there a good reason to statically initialize ntdll locks or
can they be initialized like normal locks?

Here is a patch that adds the linked list of debug info:
-------------- next part --------------
Index: dlls/ntdll/critsection.c
===================================================================
RCS file: /home/wine/wine/dlls/ntdll/critsection.c,v
retrieving revision 1.27
diff -u -r1.27 critsection.c
--- dlls/ntdll/critsection.c	6 Sep 2004 20:26:23 -0000	1.27
+++ dlls/ntdll/critsection.c	10 Sep 2004 16:29:14 -0000
@@ -36,6 +36,51 @@
 WINE_DEFAULT_DEBUG_CHANNEL(ntdll);
 WINE_DECLARE_DEBUG_CHANNEL(relay);
 
+static LIST_ENTRY RtlCriticalSectionList;
+static CRITICAL_SECTION RtlCriticalSectionLock;
+static CRITICAL_SECTION_DEBUG RtlCriticalSectionDebug =
+{
+    0, 0, &RtlCriticalSectionLock,
+    { &RtlCriticalSectionDebug.ProcessLocksList,
+      &RtlCriticalSectionDebug.ProcessLocksList },
+      0, 0, { 0, (DWORD)(__FILE__ ": RtlCriticalSectionLock") }
+};
+static CRITICAL_SECTION RtlCriticalSectionLock = { &RtlCriticalSectionDebug, -1, 0, 0, 0, 0 };
+
+void critsection_init()
+{
+    InitializeListHead( &RtlCriticalSectionList );
+    InsertHeadList( &RtlCriticalSectionList, &RtlCriticalSectionDebug.ProcessLocksList );
+}
+
+static void CRIT_Dump()
+{
+    int i = 0;
+    PLIST_ENTRY mark, entry;
+    PCRITICAL_SECTION_DEBUG debug;
+                                                                                
+    RtlEnterCriticalSection( &RtlCriticalSectionLock );
+                                                                                
+    mark = &RtlCriticalSectionList;
+    for (entry = mark->Flink; entry != mark; entry = entry->Flink)
+    {
+        debug = CONTAINING_RECORD(entry, CRITICAL_SECTION_DEBUG, ProcessLocksList);
+
+        TRACE("RtlCriticalSectionDebug[%d]:\n", i++);
+        TRACE("  Type = %04x\n", debug->Type);
+        TRACE("  CreatorBackTraceIndex = %04x\n", debug->CreatorBackTraceIndex);
+        TRACE("  CriticalSection = %p\n", debug->CriticalSection);
+        TRACE("  ProcessLocksList.Flink = %p\n", debug->ProcessLocksList.Flink);
+        TRACE("  ProcessLocksList.Blink = %p\n", debug->ProcessLocksList.Blink);
+        TRACE("  EntryCount = %ld\n", debug->EntryCount);
+        TRACE("  ContentionCount = %ld\n", debug->ContentionCount);
+        TRACE("  Spare[0] = %08lx\n", debug->Spare[0]);
+        TRACE("  Spare[1] = %08lx (%s)\n", debug->Spare[1], debugstr_a((char *)debug->Spare[1]));
+    }
+                                                                                
+    RtlLeaveCriticalSection( &RtlCriticalSectionLock );
+}
+
 inline static LONG interlocked_inc( PLONG dest )
 {
     return interlocked_xchg_add( dest, 1 ) + 1;
@@ -126,12 +171,16 @@
             crit->DebugInfo->Type = 0;
             crit->DebugInfo->CreatorBackTraceIndex = 0;
             crit->DebugInfo->CriticalSection = crit;
-            crit->DebugInfo->ProcessLocksList.Blink = &(crit->DebugInfo->ProcessLocksList);
-            crit->DebugInfo->ProcessLocksList.Flink = &(crit->DebugInfo->ProcessLocksList);
             crit->DebugInfo->EntryCount = 0;
             crit->DebugInfo->ContentionCount = 0;
             crit->DebugInfo->Spare[0] = 0;
             crit->DebugInfo->Spare[1] = 0;
+            RtlEnterCriticalSection( &RtlCriticalSectionLock );
+            InsertTailList( &RtlCriticalSectionList, &(crit->DebugInfo->ProcessLocksList) );
+            RtlLeaveCriticalSection( &RtlCriticalSectionLock );
+
+            if (TRACE_ON(ntdll))
+                CRIT_Dump();
         }
     }
     crit->LockCount      = -1;
@@ -195,12 +244,19 @@
     crit->LockSemaphore  = 0;
     if (crit->DebugInfo)
     {
+        RtlEnterCriticalSection( &RtlCriticalSectionLock );
+        RemoveEntryList( &(crit->DebugInfo->ProcessLocksList) );
+        RtlLeaveCriticalSection( &RtlCriticalSectionLock );
+
         /* only free the ones we made in here */
         if (!crit->DebugInfo->Spare[1])
         {
             RtlFreeHeap( GetProcessHeap(), 0, crit->DebugInfo );
             crit->DebugInfo = NULL;
         }
+
+        if (TRACE_ON(ntdll))
+            CRIT_Dump();
     }
     return STATUS_SUCCESS;
 }
Index: dlls/ntdll/ntdll_misc.h
===================================================================
RCS file: /home/wine/wine/dlls/ntdll/ntdll_misc.h,v
retrieving revision 1.48
diff -u -r1.48 ntdll_misc.h
--- dlls/ntdll/ntdll_misc.h	15 Jul 2004 22:07:21 -0000	1.48
+++ dlls/ntdll/ntdll_misc.h	10 Sep 2004 16:29:14 -0000
@@ -50,6 +50,7 @@
 extern void debug_init(void);
 extern void thread_init(void);
 extern void virtual_init(void);
+extern void critsection_init(void);
 
 /* server support */
 extern void server_init_process(void);
Index: dlls/ntdll/thread.c
===================================================================
RCS file: /home/wine/wine/dlls/ntdll/thread.c,v
retrieving revision 1.21
diff -u -r1.21 thread.c
--- dlls/ntdll/thread.c	23 Aug 2004 18:52:54 -0000	1.21
+++ dlls/ntdll/thread.c	10 Sep 2004 16:29:14 -0000
@@ -134,6 +134,7 @@
 
     debug_info.str_pos = debug_info.strings;
     debug_info.out_pos = debug_info.output;
+    critsection_init();
     debug_init();
     virtual_init();