Skip to content

Commit

Permalink
Release global lock before throwing exceptions from the Unix WaitSyst…
Browse files Browse the repository at this point in the history 
…em (dotnet#71626)

The exception throwing and handling can enter wait system recursively that will lead to deadlock.

Fixes dotnet#70010
  • Loading branch information
@jkotas
jkotas authored Jul 5, 2022
1 parent 6b0d373 commit a4f2109
Show file tree
Hide file tree
Showing 3 changed files with 118 additions and 97 deletions.
9 changes: 5 additions & 4 deletions ...ibraries/System.Private.CoreLib/src/System/Threading/WaitSubsystem.ThreadWaitInfo.Unix.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ private bool IsWaiting

/// <summary>
/// Callers must ensure to clear the array after use. Once <see cref="RegisterWait(int, bool, bool)"/> is called (followed
/// by a call to <see cref="Wait(int, bool, bool)"/>, the array will be cleared automatically.
/// by a call to <see cref="Wait(int, bool, bool, ref LockHolder)"/>, the array will be cleared automatically.
/// </summary>
public WaitableObject?[] GetWaitedObjectArray(int requiredCapacity)
{
Expand Down Expand Up @@ -276,7 +276,7 @@ private int ProcessSignaledWaitState()
}
}

public int Wait(int timeoutMilliseconds, bool interruptible, bool isSleep)
public int Wait(int timeoutMilliseconds, bool interruptible, bool isSleep, ref LockHolder lockHolder)
{
if (isSleep)
{
Expand All @@ -299,7 +299,7 @@ public int Wait(int timeoutMilliseconds, bool interruptible, bool isSleep)
_waitMonitor.Acquire();
if (!isSleep)
{
s_lock.Release();
lockHolder.Dispose();
}

Debug.Assert(_waitedObjectIndexThatSatisfiedWait < 0);
Expand Down Expand Up @@ -404,11 +404,12 @@ public static void Sleep(int timeoutMilliseconds, bool interruptible)
return;
}

LockHolder dummyLockHolder = default;
int waitResult =
Thread
.CurrentThread
.WaitInfo
.Wait(timeoutMilliseconds, interruptible, isSleep: true);
.Wait(timeoutMilliseconds, interruptible, isSleep: true, ref dummyLockHolder);
Debug.Assert(waitResult == WaitHandle.WaitTimeout);
}

Expand Down
93 changes: 57 additions & 36 deletions src/libraries/System.Private.CoreLib/src/System/Threading/WaitSubsystem.Unix.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -120,9 +120,44 @@ internal static partial class WaitSubsystem
{
private static readonly LowLevelLock s_lock = new LowLevelLock();

// Exception handling may use the WaitSubsystem. It means that we need to release the WaitSubsystem
// lock before throwing any exceptions to avoid deadlocks. LockHolder allows us to pass the lock state
// around and keep track of whether the lock still needs to be released.
public struct LockHolder
{
private LowLevelLock? _lock;

public LockHolder(LowLevelLock l)
{
l.Acquire();
_lock = l;
}

public void Dispose()
{
if (_lock != null)
{
_lock.Release();
_lock = null;
}
}
}

private static SafeWaitHandle NewHandle(WaitableObject waitableObject)
{
IntPtr handle = HandleManager.NewHandle(waitableObject);
IntPtr handle = IntPtr.Zero;
try
{
handle = HandleManager.NewHandle(waitableObject);
}
finally
{
if (handle == IntPtr.Zero)
{
waitableObject.OnDeleteHandle();
}
}

SafeWaitHandle? safeWaitHandle = null;
try
{
Expand Down Expand Up @@ -172,8 +207,7 @@ public static SafeWaitHandle NewMutex(bool initiallyOwned)
// between adding the mutex to the named object table and initially acquiring it.
// To avoid the possibility of another thread retrieving the mutex via its name
// before we managed to acquire it, we perform both steps while holding s_lock.
s_lock.Acquire();
bool holdingLock = true;
LockHolder lockHolder = new LockHolder(s_lock);
try
{
WaitableObject? waitableObject = WaitableObject.CreateNamedMutex_Locked(name, out createdNew);
Expand All @@ -191,18 +225,13 @@ public static SafeWaitHandle NewMutex(bool initiallyOwned)
// by the thread. See <see cref="ThreadWaitInfo.LockedMutexesHead"/>. So, acquire the lock only after all
// possibilities for exceptions have been exhausted.
ThreadWaitInfo waitInfo = Thread.CurrentThread.WaitInfo;
int status = waitableObject.Wait_Locked(waitInfo, timeoutMilliseconds: 0, interruptible: false, prioritize: false);
int status = waitableObject.Wait_Locked(waitInfo, timeoutMilliseconds: 0, interruptible: false, prioritize: false, ref lockHolder);
Debug.Assert(status == 0);
// Wait_Locked has already released s_lock, so we no longer hold it here.
holdingLock = false;
return safeWaitHandle;
}
finally
{
if (holdingLock)
{
s_lock.Release();
}
lockHolder.Dispose();
}
}

Expand All @@ -227,14 +256,14 @@ public static void SetEvent(WaitableObject waitableObject)
{
Debug.Assert(waitableObject != null);

s_lock.Acquire();
LockHolder lockHolder = new LockHolder(s_lock);
try
{
waitableObject.SignalEvent();
waitableObject.SignalEvent(ref lockHolder);
}
finally
{
s_lock.Release();
lockHolder.Dispose();
}
}

Expand All @@ -247,14 +276,14 @@ public static void ResetEvent(WaitableObject waitableObject)
{
Debug.Assert(waitableObject != null);

s_lock.Acquire();
LockHolder lockHolder = new LockHolder(s_lock);
try
{
waitableObject.UnsignalEvent();
waitableObject.UnsignalEvent(ref lockHolder);
}
finally
{
s_lock.Release();
lockHolder.Dispose();
}
}

Expand All @@ -269,14 +298,14 @@ public static int ReleaseSemaphore(WaitableObject waitableObject, int count)
Debug.Assert(waitableObject != null);
Debug.Assert(count > 0);

s_lock.Acquire();
LockHolder lockHolder = new LockHolder(s_lock);
try
{
return waitableObject.SignalSemaphore(count);
return waitableObject.SignalSemaphore(count, ref lockHolder);
}
finally
{
s_lock.Release();
lockHolder.Dispose();
}
}

Expand All @@ -289,14 +318,14 @@ public static void ReleaseMutex(WaitableObject waitableObject)
{
Debug.Assert(waitableObject != null);

s_lock.Acquire();
LockHolder lockHolder = new LockHolder(s_lock);
try
{
waitableObject.SignalMutex();
waitableObject.SignalMutex(ref lockHolder);
}
finally
{
s_lock.Release();
lockHolder.Dispose();
}
}

Expand Down Expand Up @@ -412,38 +441,30 @@ public static int SignalAndWait(
Debug.Assert(timeoutMilliseconds >= -1);

ThreadWaitInfo waitInfo = Thread.CurrentThread.WaitInfo;
bool waitCalled = false;
s_lock.Acquire();
LockHolder lockHolder = new LockHolder(s_lock);
try
{
// A pending interrupt does not signal the specified handle
if (interruptible && waitInfo.CheckAndResetPendingInterrupt)
{
lockHolder.Dispose();
throw new ThreadInterruptedException();
}

try
{
waitableObjectToSignal.Signal(1);
waitableObjectToSignal.Signal(1, ref lockHolder);
}
catch (SemaphoreFullException ex)
{
s_lock.VerifyIsNotLocked();
throw new InvalidOperationException(SR.Threading_WaitHandleTooManyPosts, ex);
}
waitCalled = true;
return waitableObjectToWaitOn.Wait_Locked(waitInfo, timeoutMilliseconds, interruptible, prioritize);
return waitableObjectToWaitOn.Wait_Locked(waitInfo, timeoutMilliseconds, interruptible, prioritize, ref lockHolder);
}
finally
{
// Once the wait function is called, it will release the lock
if (waitCalled)
{
s_lock.VerifyIsNotLocked();
}
else
{
s_lock.Release();
}
lockHolder.Dispose();
}
}

Expand Down
Loading

0 comments on commit a4f2109

Please sign in to comment.