Skip to content

Commit

Permalink
Recover ReentrantSemaphore after JTF throws an exception
Browse files Browse the repository at this point in the history 
We already had a try/finally block, but it assumed JTF.RunAsync itself wouldn't throw an exception. In some cases it *does* throw an exception (`OutOfMemoryException` for example) and it's critical to avoiding deadlocks later on that we exit the semaphore we entered in this case so we do not deadlock later.

Fixes #843
  • Loading branch information
@AArnott
AArnott committed May 4, 2021
1 parent 4de5495 commit 9a5e278
Showing 1 changed file with 110 additions and 110 deletions.
220 changes: 110 additions & 110 deletions src/Microsoft.VisualStudio.Threading/ReentrantSemaphore.cs
View file
Expand Up @@ -352,9 +352,9 @@ public override async Task ExecuteAsync(Func<Task> operation, CancellationToken
}
}

await this.ExecuteCoreAsync(async delegate
try
{
try
await this.ExecuteCoreAsync(async delegate
{
if (this.IsJoinableTaskAware(out JoinableTaskFactory? joinableTaskFactory, out _))
{
Expand All @@ -376,12 +376,12 @@ public override async Task ExecuteAsync(Func<Task> operation, CancellationToken
}

await operation().ConfigureAwaitRunInline();
}
finally
{
DisposeReleaserNoThrow(releaser);
}
});
});
}
finally
{
DisposeReleaserNoThrow(releaser);
}
}

/// <inheritdoc />
Expand Down Expand Up @@ -416,9 +416,9 @@ public override async ValueTask<T> ExecuteAsync<T>(Func<ValueTask<T>> operation,
}
}

return await this.ExecuteCoreAsync(async delegate
try
{
try
return await this.ExecuteCoreAsync(async delegate
{
if (this.IsJoinableTaskAware(out JoinableTaskFactory? joinableTaskFactory, out _))
{
Expand All @@ -440,12 +440,12 @@ public override async ValueTask<T> ExecuteAsync<T>(Func<ValueTask<T>> operation,
}

return await operation().ConfigureAwait(true);
}
finally
{
DisposeReleaserNoThrow(releaser);
}
});
});
}
finally
{
DisposeReleaserNoThrow(releaser);
}
}
}

Expand Down Expand Up @@ -515,9 +515,9 @@ public override async Task ExecuteAsync(Func<Task> operation, CancellationToken
}
}

await this.ExecuteCoreAsync(async delegate
try
{
try
await this.ExecuteCoreAsync(async delegate
{
if (this.IsJoinableTaskAware(out JoinableTaskFactory? joinableTaskFactory, out _))
{
Expand All @@ -540,19 +540,19 @@ public override async Task ExecuteAsync(Func<Task> operation, CancellationToken

this.reentrancyDetection.Value = ownedBox = new StrongBox<bool>(true);
await operation().ConfigureAwaitRunInline();
}
finally
});
}
finally
{
// Make it clear to any forks of our ExecutionContexxt that the semaphore is no longer owned.
// Null check incase the switch to UI thread was cancelled.
if (ownedBox is object)
{
// Make it clear to any forks of our ExecutionContexxt that the semaphore is no longer owned.
// Null check incase the switch to UI thread was cancelled.
if (ownedBox is object)
{
ownedBox.Value = false;
}

DisposeReleaserNoThrow(releaser);
ownedBox.Value = false;
}
});

DisposeReleaserNoThrow(releaser);
}
}

/// <inheritdoc />
Expand Down Expand Up @@ -594,9 +594,9 @@ public override async ValueTask<T> ExecuteAsync<T>(Func<ValueTask<T>> operation,
}
}

return await this.ExecuteCoreAsync(async delegate
try
{
try
return await this.ExecuteCoreAsync(async delegate
{
if (this.IsJoinableTaskAware(out JoinableTaskFactory? joinableTaskFactory, out _))
{
Expand All @@ -619,19 +619,19 @@ public override async ValueTask<T> ExecuteAsync<T>(Func<ValueTask<T>> operation,

this.reentrancyDetection.Value = ownedBox = new StrongBox<bool>(true);
return await operation().ConfigureAwait(true);
}
finally
});
}
finally
{
// Make it clear to any forks of our ExecutionContexxt that the semaphore is no longer owned.
// Null check incase the switch to UI thread was cancelled.
if (ownedBox is object)
{
// Make it clear to any forks of our ExecutionContexxt that the semaphore is no longer owned.
// Null check incase the switch to UI thread was cancelled.
if (ownedBox is object)
{
ownedBox.Value = false;
}

DisposeReleaserNoThrow(releaser);
ownedBox.Value = false;
}
});

DisposeReleaserNoThrow(releaser);
}
}

/// <inheritdoc />
Expand Down Expand Up @@ -725,11 +725,11 @@ public override async Task ExecuteAsync(Func<Task> operation, CancellationToken
releaser = default;
}

await this.ExecuteCoreAsync(async delegate
bool pushed = false;
var pushedReleaser = new StrongBox<AsyncSemaphore.Releaser>(releaser);
try
{
bool pushed = false;
var pushedReleaser = new StrongBox<AsyncSemaphore.Releaser>(releaser);
try
await this.ExecuteCoreAsync(async delegate
{
if (this.IsJoinableTaskAware(out JoinableTaskFactory? joinableTaskFactory, out _))
{
Expand Down Expand Up @@ -760,33 +760,33 @@ public override async Task ExecuteAsync(Func<Task> operation, CancellationToken
}

await operation().ConfigureAwaitRunInline();
}
finally
});
}
finally
{
try
{
try
if (pushed)
{
if (pushed)
lock (reentrantStack)
{
lock (reentrantStack)
StrongBox<AsyncSemaphore.Releaser>? poppedReleaser = reentrantStack.Pop();
if (!object.ReferenceEquals(poppedReleaser, pushedReleaser))
{
StrongBox<AsyncSemaphore.Releaser>? poppedReleaser = reentrantStack.Pop();
if (!object.ReferenceEquals(poppedReleaser, pushedReleaser))
{
// When the semaphore faults, we will drain and throw for awaiting tasks one by one.
this.faulted = true;
// When the semaphore faults, we will drain and throw for awaiting tasks one by one.
this.faulted = true;
#pragma warning disable CA2219 // Do not raise exceptions in finally clauses
throw Verify.FailOperation(Strings.SemaphoreStackNestingViolated, ReentrancyMode.Stack);
throw Verify.FailOperation(Strings.SemaphoreStackNestingViolated, ReentrancyMode.Stack);
#pragma warning restore CA2219 // Do not raise exceptions in finally clauses
}
}
}
}
finally
{
DisposeReleaserNoThrow(releaser);
}
}
});
finally
{
DisposeReleaserNoThrow(releaser);
}
}
}

/// <inheritdoc />
Expand Down Expand Up @@ -841,11 +841,11 @@ public override async ValueTask<T> ExecuteAsync<T>(Func<ValueTask<T>> operation,
releaser = default;
}

return await this.ExecuteCoreAsync(async delegate
bool pushed = false;
var pushedReleaser = new StrongBox<AsyncSemaphore.Releaser>(releaser);
try
{
bool pushed = false;
var pushedReleaser = new StrongBox<AsyncSemaphore.Releaser>(releaser);
try
return await this.ExecuteCoreAsync(async delegate
{
if (this.IsJoinableTaskAware(out JoinableTaskFactory? joinableTaskFactory, out _))
{
Expand Down Expand Up @@ -876,33 +876,33 @@ public override async ValueTask<T> ExecuteAsync<T>(Func<ValueTask<T>> operation,
}

return await operation().ConfigureAwait(true);
}
finally
});
}
finally
{
try
{
try
if (pushed)
{
if (pushed)
lock (reentrantStack)
{
lock (reentrantStack)
StrongBox<AsyncSemaphore.Releaser>? poppedReleaser = reentrantStack.Pop();
if (!object.ReferenceEquals(poppedReleaser, pushedReleaser))
{
StrongBox<AsyncSemaphore.Releaser>? poppedReleaser = reentrantStack.Pop();
if (!object.ReferenceEquals(poppedReleaser, pushedReleaser))
{
// When the semaphore faults, we will drain and throw for awaiting tasks one by one.
this.faulted = true;
// When the semaphore faults, we will drain and throw for awaiting tasks one by one.
this.faulted = true;
#pragma warning disable CA2219 // Do not raise exceptions in finally clauses
throw Verify.FailOperation(Strings.SemaphoreStackNestingViolated, ReentrancyMode.Stack);
throw Verify.FailOperation(Strings.SemaphoreStackNestingViolated, ReentrancyMode.Stack);
#pragma warning restore CA2219 // Do not raise exceptions in finally clauses
}
}
}
}
finally
{
DisposeReleaserNoThrow(releaser);
}
}
});
finally
{
DisposeReleaserNoThrow(releaser);
}
}
}

/// <inheritdoc />
Expand Down Expand Up @@ -990,10 +990,10 @@ public override async Task ExecuteAsync(Func<Task> operation, CancellationToken
releaser = default;
}

await this.ExecuteCoreAsync(async delegate
bool pushed = false;
try
{
bool pushed = false;
try
await this.ExecuteCoreAsync(async delegate
{
if (this.IsJoinableTaskAware(out JoinableTaskFactory? joinableTaskFactory, out _))
{
Expand Down Expand Up @@ -1022,20 +1022,20 @@ public override async Task ExecuteAsync(Func<Task> operation, CancellationToken
}

await operation().ConfigureAwaitRunInline();
}
finally
});
}
finally
{
if (pushed)
{
if (pushed)
lock (reentrantStack)
{
lock (reentrantStack)
{
releaser = reentrantStack.Pop();
}
releaser = reentrantStack.Pop();
}

DisposeReleaserNoThrow(releaser);
}
});

DisposeReleaserNoThrow(releaser);
}
}

/// <inheritdoc />
Expand Down Expand Up @@ -1086,10 +1086,10 @@ public override async ValueTask<T> ExecuteAsync<T>(Func<ValueTask<T>> operation,
releaser = default;
}

return await this.ExecuteCoreAsync(async delegate
bool pushed = false;
try
{
bool pushed = false;
try
return await this.ExecuteCoreAsync(async delegate
{
if (this.IsJoinableTaskAware(out JoinableTaskFactory? joinableTaskFactory, out _))
{
Expand Down Expand Up @@ -1118,20 +1118,20 @@ public override async ValueTask<T> ExecuteAsync<T>(Func<ValueTask<T>> operation,
}

return await operation().ConfigureAwait(true);
}
finally
});
}
finally
{
if (pushed)
{
if (pushed)
lock (reentrantStack)
{
lock (reentrantStack)
{
releaser = reentrantStack.Pop();
}
releaser = reentrantStack.Pop();
}

DisposeReleaserNoThrow(releaser);
}
});

DisposeReleaserNoThrow(releaser);
}
}

/// <inheritdoc />
Expand Down

0 comments on commit 9a5e278

Please sign in to comment.