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

Update IConsumerRegister.Default.cs to make dispose thread safe #1438

Merged

Conversation

blashbul
Copy link
Contributor

Add memory barrier to _disposed field to make Dispose call thread safe

Description:

When doing Integration testing with WebApplicationFactory Class using xunit, tests tear down dispose twice.
One by CancelationToken, with stop method. This first call make CancelationTokenSource disposed, and every thing is fine.
After that WebApplicationFactory is then disposed, and also call ConsumerRegister.Dispose.
Due to race condition, Pulse method is called a second time and try to dispose already disposed CancellationTokenSource.

It seems to be related to dotnet/aspnetcore#40271

Changes:

  • DotNetCore.CAP.Internal.ConsumerRegister.Dispose have been made thread safe

Affected components:

  • DotNetCore.CAP.Internal.ConsumerRegister

How to test:

Add an Integration Test project with a WebApplicationFactory, run in Debug mode, exception is logged in Debug output.
Depending your host setup (old fashioned with program and startup class), exception causes test failure with error :
Message:
[Test Collection Cleanup Failure (Data Integration Tests Collection)]: System.ObjectDisposedException : The CancellationTokenSource has been disposed.

  Stack Trace:

CancellationTokenSource.Cancel()
ConsumerRegister.Pulse() line 109
ConsumerRegister.Dispose() line 96
ServiceProviderEngineScope.DisposeAsync()
--- End of stack trace from previous location ---
Host.g__DisposeAsync|16_0(Object o)
Host.DisposeAsync()
Host.Dispose()
WebApplicationFactory1.DisposeAsync() WebApplicationFactory1.Dispose(Boolean disposing)
WebApplicationFactory`1.Dispose()

Checklist:

  • I have tested my changes locally
  • My changes follow the project's code style guidelines

Add memory barrier to _disposed field to make Dispose call thread safe
Copy link
Member

@yang-xiaodong yang-xiaodong left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, thanks for your PR!

@yang-xiaodong yang-xiaodong merged commit 665390f into dotnetcore:master Nov 23, 2023
2 checks passed
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

Successfully merging this pull request may close these issues.

2 participants