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

4.x: Make RefCountDisposable lock-free #574

Merged
merged 4 commits into from
Jun 6, 2018

Conversation

akarnokd
Copy link
Collaborator

@akarnokd akarnokd commented Jun 5, 2018

This PR changes the RefCountDisposable to use lock-free algorithms which are based on atomic conditional state changes: changes in the number of child disposables and whether the main disposable can be disposed.

It works by using bit 31 in the _count as the indicator for disposing the main underlying disposable once all child disposables have been disposed. Bits 30-0 are then the number of active child disposables. Since the field is still int this allows atomic CAS operations on this composite bit-structure.

@danielcweber
Copy link
Collaborator

Nice.


// Increment the active count by one, works because the increment
// won't affect bit 31
var u = Interlocked.CompareExchange(ref _count, cnt + 1, cnt);
Copy link
Collaborator

@danielcweber danielcweber Jun 6, 2018

Choose a reason for hiding this comment

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

The behaviour is excatly as in the locked version before, after int.MaxValue it will wrap and lead to strange behaviour (actually, the new version will act as if disposed). We should take the opportunity and not just Debug.Assert but throw something.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Shouldn't the debug assert throw always on failure? Besides, I'm not sure we can support 2 billion child subscribers anyway.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Debug.Assert will not be compiled into Release builds. 2 billion child subscribers are possible, tested this morning.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We could expand to long, allowing far more child disposables.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't really think anybody will hit that number, let's for now just make sure it throws instead of wrapping.

@danielcweber danielcweber merged commit bc340c6 into dotnet:master Jun 6, 2018
@akarnokd akarnokd deleted the RefCountDisposableLockFree branch June 6, 2018 21:04
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