alloc: clarify comment on ArcInner::weak lock sentinel#156787
Merged
rust-bors[bot] merged 1 commit intoJun 13, 2026
Conversation
rustbot
added
S-waiting-on-review
Collaborator
|
rustbot has assigned @Mark-Simulacrum. Use Why was this reviewer chosen?The reviewer was selected based on:
|
joboet
reviewed
May 21, 2026
Comment on lines
+394
to
+395
| // `Arc::get_mut`) to atomically confirm that only the implicit weak | ||
| // reference exists before checking the strong count for uniqueness. |
Member
There was a problem hiding this comment.
I think this could be phrased more precisely. The implicit-weak-reference check is trivial to achieve atomically, it's just that the entire operation (both the strong count and weak count comparison) has to be atomic, and the only way to achieve that is by locking one of the counts.
Member
|
This looks fine to me. Could you please squash your commits? |
Member
|
@rustbot author |
rustbot
added
S-waiting-on-author
Collaborator
|
Reminder, once the PR becomes ready for a review, use |
Explain that the weak count doubles as a lock taken by get_mut and downgrade, and that the value 1 is the implicit weak reference owned by the strong references rather than an absence of weak handles.
LeSingh1
force-pushed
the
fix-arc-inner-weak-comment
branch
from
7d9e442 to
2be9d51
Compare
Contributor
Author
|
Squashed into a single commit. @rustbot ready |
rustbot
added
S-waiting-on-review
Member
|
Great, thanks! |
Contributor
rust-bors
Bot
added
S-waiting-on-bors
rust-timer
added a commit
that referenced
this pull request
Jun 13, 2026
Rollup merge of #156787 - LeSingh1:fix-arc-inner-weak-comment, r=joboet alloc: clarify comment on ArcInner::weak lock sentinel The comment on the `weak` field of `ArcInner` currently states that the `usize::MAX` sentinel locks "the ability to upgrade weak pointers or downgrade strong ones", and that this is used to avoid races in `make_mut` and `get_mut`. Both halves of that description are inaccurate. `Weak::upgrade` never inspects the weak count, so the lock has no effect on it. The operation only CASes on the strong count and is happy to succeed while the weak count is locked. `Arc::make_mut` also does not participate in the locking protocol, and the function body already contains a comment noting that observing `usize::MAX` for the weak count there is impossible because only a thread holding a strong reference can take the lock. The lock is actually only taken in `Arc::is_unique` (which backs `Arc::get_mut`), and the only operation that observes and waits on it is `Arc::downgrade`, which spins while `weak == usize::MAX` before attempting its CAS to create a new `Weak`. This PR rewrites the comment to describe what the lock is and what it actually prevents. Closes #144296
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
4 participants
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.
The comment on the
weakfield ofArcInnercurrently states that theusize::MAXsentinel locks "the ability to upgrade weak pointers or downgrade strong ones", and that this is used to avoid races inmake_mutandget_mut. Both halves of that description are inaccurate.Weak::upgradenever inspects the weak count, so the lock has no effect on it. The operation only CASes on the strong count and is happy to succeed while the weak count is locked.Arc::make_mutalso does not participate in the locking protocol, and the function body already contains a comment noting that observingusize::MAXfor the weak count there is impossible because only a thread holding a strong reference can take the lock.The lock is actually only taken in
Arc::is_unique(which backsArc::get_mut), and the only operation that observes and waits on it isArc::downgrade, which spins whileweak == usize::MAXbefore attempting its CAS to create a newWeak.This PR rewrites the comment to describe what the lock is and what it actually prevents.
Closes #144296