Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Regression in 6.2.3: Semaphore Disposed Exception thrown in AsyncConsumerWorkService #1153

Closed
ashneilson opened this issue Feb 23, 2022 · 4 comments
Assignees
Milestone

Comments

@ashneilson
Copy link
Contributor

Refer to the original issue for context #1014

Problem

When under load, stopping the AsyncConsumerWorkService can result in System.ObjectDisposedException being thrown due to a Disposed _limiter.

The Discarded Tasks HandleConcurrent started in LoopWithConcurrency may still be running after the finally block disposes of the _limiter Semaphore.

private async Task LoopWithConcurrency(CancellationToken cancellationToken)
{
try
{
while (await _channel.Reader.WaitToReadAsync(cancellationToken).ConfigureAwait(false))
{
while (_channel.Reader.TryRead(out Work work))
{
// Do a quick synchronous check before we resort to async/await with the state-machine overhead.
if (!_limiter.Wait(0))
{
await _limiter.WaitAsync(cancellationToken).ConfigureAwait(false);
}
_ = HandleConcurrent(work, _model, _limiter);
}
}
}
catch (OperationCanceledException)
{
// ignored
}
finally
{
_limiter?.Dispose();
}
}

When the HandleConcurrent Tasks attempt to Release the Semaphore, the System.ObjectDisposedException is thrown.

}
finally
{
work.PostExecute();
limiter.Release();
}

The issue doesn't occur in 6.2.2 but does in 6.2.3. It appears the PR #1113 might be the culprit.

Recommended Fix

I'd be more than happy to submit a PR with a fix if that's the desired approach.

@michaelklishin
Copy link
Member

@ashneilson please submit a PR if you understand what needs to be done (not just a revert).

ashneilson added a commit to ricado-group/rabbitmq-dotnet-client that referenced this issue Feb 23, 2022
@ashneilson
Copy link
Contributor Author

@michaelklishin Thanks for the quick reply! PR submitted as requested

@lukebakken
Copy link
Contributor

Thanks I'll get to this today.

michaelklishin added a commit that referenced this issue Feb 23, 2022
…ception

Fix for #1153 - Semaphore disposed before discarded tasks have finished
@lukebakken
Copy link
Contributor

Fixed by #1154 and will ship in 6.2.4.

@lukebakken lukebakken added this to the 6.2.4 milestone Feb 23, 2022
@lukebakken lukebakken self-assigned this Feb 23, 2022
lukebakken pushed a commit that referenced this issue Feb 23, 2022
…ception

Fix for #1153 - Semaphore disposed before discarded tasks have finished

(cherry picked from commit 0c95330)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants