We are currently migrating Bugzilla to GitHub issues.
Any changes made to the bug tracker now will be lost, so please do not post new bugs or make changes to them.
When we're done, all bug URLs will redirect to their equivalent location on the new bug tracker.

Bug 5537

Summary: [PATCH] Deadlock in SDL_CondWait() with SRW lock implementation
Product: SDL Reporter: Cameron Gutman <cameron.gutman>
Component: threadAssignee: Sam Lantinga <slouken>
Status: RESOLVED FIXED QA Contact: Sam Lantinga <slouken>
Severity: critical    
Priority: P2    
Version: HG 2.0   
Hardware: x86   
OS: Windows 10   
Attachments: Patch

Description Cameron Gutman 2021-02-09 00:31:07 UTC
When SleepConditionVariableSRW() releases the SRW lock internally, it causes our SDL_mutex_srw state to become inconsistent. The lock is unowned yet inside, the owner is still the sleeping thread and more importantly the owner count is still 1.

The next time someone acquires the lock, they will bump the owner count from 1 to 2. At that point, the lock is hosed. From the internal lock state, it looks to us like that owner has acquired the lock recursively, even though they have not. When they call SDL_UnlockMutex(), it will see the owner count > 0 and not call ReleaseSRWLockExclusive().

Now when someone calls SDL_CondSignal(), SleepConditionVariableSRW() will start the wakeup process by attempting to re-acquire the SRW lock. This will deadlock because the lock was never released after the other thread had used it. The thread waiting on the condition variable will never be able to wake up, even if the SDL_CondWaitTimeout() function is used and the timeout expires.

Reproducing it is a simple as:

Thread 1:
SDL_LockMutex(mutex);
SDL_CondWait(cond, mutex); <- Deadlocks in here
SDL_UnlockMutex(mutex);

Thread 2:
SDL_LockMutex(mutex);
SDL_UnlockMutex(mutex); <- Underlying SRW lock is still held after this
SDL_CondSignal(cond);
Comment 1 Cameron Gutman 2021-02-09 00:32:05 UTC
Created attachment 4782 [details]
Patch
Comment 2 Sam Lantinga 2021-02-09 00:57:25 UTC
Patch added, thanks!
https://hg.libsdl.org/SDL/rev/9062f074edfd