Skip to content

Commit

Permalink
[RTL] Update CountOfOwnedCriticalSections in the TEB
Browse files Browse the repository at this point in the history
Useful for debugging.
Motivation: With SMP on x64 I found a number of instances where critical sections would be left abandoned, causing lockups. From what I can tell it was exceptions inside rpcrt4, which leave the process in a blocked state. Might or might not be related to x64 / SMP.

For real value, you still need to put checks at certain places manually, but this is not super straight forward, because there can be false positives, e.g. when a process is terminated due to an exception, where the abandoned lock is acceptable, and we have this during testing. It's difficult to 100% distinguish this from silent and very bad lock leaks.

Problematic code:

    __try
    {
        SomeFunction(); // throws an exception with a CS held, e.g. heap code
    }
    __except(1)
    {
        DPRINT1("Oops. let's just pretend it's all ok!\n");
    }
  • Loading branch information
tkreuzer committed Jan 16, 2025
1 parent 7bb1266 commit c698009
Showing 1 changed file with 4 additions and 1 deletion.
5 changes: 4 additions & 1 deletion sdk/lib/rtl/critical.c
Original file line number Diff line number Diff line change
Expand Up @@ -521,6 +521,7 @@ RtlEnterCriticalSection(PRTL_CRITICAL_SECTION CriticalSection)
*/
CriticalSection->OwningThread = Thread;
CriticalSection->RecursionCount = 1;
NtCurrentTeb()->CountOfOwnedCriticalSections++;
return STATUS_SUCCESS;
}

Expand Down Expand Up @@ -800,6 +801,7 @@ RtlLeaveCriticalSection(PRTL_CRITICAL_SECTION CriticalSection)
* See comment above.
*/
CriticalSection->OwningThread = 0;
NtCurrentTeb()->CountOfOwnedCriticalSections--;

/* Was someone wanting us? This needs to be done atomically. */
if (-1 != InterlockedDecrement(&CriticalSection->LockCount))
Expand Down Expand Up @@ -837,8 +839,9 @@ RtlTryEnterCriticalSection(PRTL_CRITICAL_SECTION CriticalSection)
if (InterlockedCompareExchange(&CriticalSection->LockCount, 0, -1) == -1)
{
/* It's ours */
CriticalSection->OwningThread = NtCurrentTeb()->ClientId.UniqueThread;
CriticalSection->OwningThread = NtCurrentTeb()->ClientId.UniqueThread;
CriticalSection->RecursionCount = 1;
NtCurrentTeb()->CountOfOwnedCriticalSections++;
return TRUE;
}
else if (CriticalSection->OwningThread == NtCurrentTeb()->ClientId.UniqueThread)
Expand Down

0 comments on commit c698009

Please sign in to comment.