Skip to content

Commit

Permalink
Previous commit introduced a new race condition. It was possible that…
Browse files Browse the repository at this point in the history
… thread A could invoke `TaskQueue.Dispose()` and set `_isDisposed = 1`, which would then allow thread B to finish work setting `_held = false` but without releasing the semaphore, and then thread A would attempt `_semaphore.Wait()` entering a deadlock.
  • Loading branch information
leonardo-fernandes committed Jun 2, 2023
1 parent aabb86e commit deb351d
Showing 1 changed file with 15 additions and 8 deletions.
23 changes: 15 additions & 8 deletions lib/PuppeteerSharp/Helpers/TaskQueue.cs
Original file line number Diff line number Diff line change
Expand Up @@ -52,11 +52,8 @@ internal async Task<T> Enqueue<T>(Func<Task<T>> taskGenerator)
}
finally
{
TryRelease(_semaphore);
_held.Value = false;
if (_disposed == 0)
{
_semaphore.Release();
}
}
}

Expand All @@ -70,11 +67,21 @@ internal async Task Enqueue(Func<Task> taskGenerator)
}
finally
{
TryRelease(_semaphore);
_held.Value = false;
if (_disposed == 0)
{
_semaphore.Release();
}
}
}

private void TryRelease(SemaphoreSlim semaphore)
{
try
{
semaphore.Release();
}
catch (ObjectDisposedException)
{
// If semaphore has already been disposed, then Release() will fail
// but we can safely ignore it
}
}
}
Expand Down

0 comments on commit deb351d

Please sign in to comment.