Make sure that counter is decremented even when Arc::clone panics - #4
Open
Kritzefitz wants to merge 1 commit into
Open
Make sure that counter is decremented even when Arc::clone panics#4Kritzefitz wants to merge 1 commit into
Kritzefitz wants to merge 1 commit into
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This handles the case discussed in #3 where an implementation of
Arc::clonethat panics could cause the count to increase more than it is decreased, eventually causing an overflow. To ensure the counter is decremented even in case of panics, I usescopeguard::defer.This of course implies a dependency on the scopeguard crate. If you don't want this, I've also made another version of this fix without the dependency on scopeguard. It is of course considerably more verbose and awkwardly tries to separate the counter into it's own struct, but the separation breaks down, because the counter can't really know which orderings the surrounding code needs, so now there are a bunch of Ordering parameters all over the place. Overall, feel free to merge this, or take inspiration from this or the other solution to write your own solution.
A third option would be to use the CAS loop I've mentioned in #1. That should be safe in all cases, even the practically impossible case that there are more than
usize::MAXsimultaneous threads.