[PATCH 0/2] MR2962: ntdll: Simplify critical section timeout logic.

[Zebediah Figura (＠zfigura)]

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/2962

[PATCH 1/2] ntdll: Do not throw deadlock exceptions for Wine critical sections.

[Zebediah Figura]

From: Zebediah Figura <zfigura(a)codeweavers.com>

This is actually a public debugging facility, but currently applies exclusively
to Wine internal locks. While we could expand this feature to all critical
sections, no application is known to depend on it, and the fact that it is a
debugging feature means it is not very likely we will ever find one.

As far as Wine is concerned, I believe this feature is unused. I have not heard
any mention of it in at least five years.

Moreover, we have a different tool for debugging critical section timeouts,
namely, the ERR messages in the same function. These messages provide just as
much information while being less destructive to the process state.
---
 dlls/ntdll/sync.c | 20 --------------------
 1 file changed, 20 deletions(-)

diff --git a/dlls/ntdll/sync.c b/dlls/ntdll/sync.c
index ec974ee2af0..9e4aa38ff2b 100644
--- a/dlls/ntdll/sync.c
+++ b/dlls/ntdll/sync.c
@@ -299,8 +299,6 @@ NTSTATUS WINAPI RtlDeleteCriticalSection( RTL_CRITICAL_SECTION *crit )
  */
 NTSTATUS WINAPI RtlpWaitForCriticalSection( RTL_CRITICAL_SECTION *crit )
 {
-    LONGLONG timeout = NtCurrentTeb()->Peb->CriticalSectionTimeout.QuadPart / -10000000;
-
     /* Don't allow blocking on a critical section during process termination */
     if (RtlDllShutdownInProgress())
     {
@@ -311,9 +309,7 @@ NTSTATUS WINAPI RtlpWaitForCriticalSection( RTL_CRITICAL_SECTION *crit )
 
     for (;;)
     {
-        EXCEPTION_RECORD rec;
         NTSTATUS status = wait_semaphore( crit, 5 );
-        timeout -= 5;
 
         if ( status == STATUS_TIMEOUT )
         {
@@ -322,31 +318,15 @@ NTSTATUS WINAPI RtlpWaitForCriticalSection( RTL_CRITICAL_SECTION *crit )
             ERR( "section %p %s wait timed out in thread %04lx, blocked by %04lx, retrying (60 sec)\n",
                  crit, debugstr_a(name), GetCurrentThreadId(), HandleToULong(crit->OwningThread) );
             status = wait_semaphore( crit, 60 );
-            timeout -= 60;
 
             if ( status == STATUS_TIMEOUT && TRACE_ON(relay) )
             {
                 ERR( "section %p %s wait timed out in thread %04lx, blocked by %04lx, retrying (5 min)\n",
                      crit, debugstr_a(name), GetCurrentThreadId(), HandleToULong(crit->OwningThread) );
                 status = wait_semaphore( crit, 300 );
-                timeout -= 300;
             }
         }
         if (status == STATUS_WAIT_0) break;
-
-        /* Throw exception only for Wine internal locks */
-        if (!crit_section_has_debuginfo( crit ) || !crit->DebugInfo->Spare[0]) continue;
-
-        /* only throw deadlock exception if configured timeout is reached */
-        if (timeout > 0) continue;
-
-        rec.ExceptionCode    = STATUS_POSSIBLE_DEADLOCK;
-        rec.ExceptionFlags   = 0;
-        rec.ExceptionRecord  = NULL;
-        rec.ExceptionAddress = RtlRaiseException;  /* sic */
-        rec.NumberParameters = 1;
-        rec.ExceptionInformation[0] = (ULONG_PTR)crit;
-        RtlRaiseException( &rec );
     }
     if (crit_section_has_debuginfo( crit )) crit->DebugInfo->ContentionCount++;
     return STATUS_SUCCESS;
-- 
GitLab


https://gitlab.winehq.org/wine/wine/-/merge_requests/2962

[PATCH 2/2] ntdll: Simplify critical section timeout logic.

[Zebediah Figura]

From: Zebediah Figura <zfigura(a)codeweavers.com>

Make it more consistent, as well. Currently we alternate between 5 seconds and
60; instead just make it a consistent 60 after the first wait.

When +relay is on, always wait for 300 seconds, instead of alternating between
60 and 300.
---
 dlls/ntdll/sync.c | 22 +++++++---------------
 1 file changed, 7 insertions(+), 15 deletions(-)

diff --git a/dlls/ntdll/sync.c b/dlls/ntdll/sync.c
index 9e4aa38ff2b..fa64917029a 100644
--- a/dlls/ntdll/sync.c
+++ b/dlls/ntdll/sync.c
@@ -299,6 +299,8 @@ NTSTATUS WINAPI RtlDeleteCriticalSection( RTL_CRITICAL_SECTION *crit )
  */
 NTSTATUS WINAPI RtlpWaitForCriticalSection( RTL_CRITICAL_SECTION *crit )
 {
+    unsigned int timeout = 5;
+
     /* Don't allow blocking on a critical section during process termination */
     if (RtlDllShutdownInProgress())
     {
@@ -309,24 +311,14 @@ NTSTATUS WINAPI RtlpWaitForCriticalSection( RTL_CRITICAL_SECTION *crit )
 
     for (;;)
     {
-        NTSTATUS status = wait_semaphore( crit, 5 );
+        NTSTATUS status = wait_semaphore( crit, timeout );
 
-        if ( status == STATUS_TIMEOUT )
-        {
-            const char *name = crit_section_get_name( crit );
+        if (status == STATUS_WAIT_0) break;
 
-            ERR( "section %p %s wait timed out in thread %04lx, blocked by %04lx, retrying (60 sec)\n",
-                 crit, debugstr_a(name), GetCurrentThreadId(), HandleToULong(crit->OwningThread) );
-            status = wait_semaphore( crit, 60 );
+        timeout = (TRACE_ON(relay) ? 300 : 60);
 
-            if ( status == STATUS_TIMEOUT && TRACE_ON(relay) )
-            {
-                ERR( "section %p %s wait timed out in thread %04lx, blocked by %04lx, retrying (5 min)\n",
-                     crit, debugstr_a(name), GetCurrentThreadId(), HandleToULong(crit->OwningThread) );
-                status = wait_semaphore( crit, 300 );
-            }
-        }
-        if (status == STATUS_WAIT_0) break;
+        ERR( "section %p %s wait timed out in thread %04lx, blocked by %04lx, retrying (%u sec)\n",
+             crit, debugstr_a(crit_section_get_name(crit)), GetCurrentThreadId(), HandleToULong(crit->OwningThread), timeout );
     }
     if (crit_section_has_debuginfo( crit )) crit->DebugInfo->ContentionCount++;
     return STATUS_SUCCESS;
-- 
GitLab

https://gitlab.winehq.org/wine/wine/-/merge_requests/2962