Skip to content

The comment about the weak field in ArcArcInner is imprecise #144296

@xmh0511

Description

@xmh0511
#[repr(C)]
struct ArcInner<T: ?Sized> {
    strong: Atomic<usize>,

    // the value usize::MAX acts as a sentinel for temporarily "locking" the
    // ability to upgrade weak pointers or downgrade strong ones; this is used
    // to avoid races in `make_mut` and `get_mut`.
    weak: Atomic<usize>,

    data: T,
}

By examining the implementation of Weak::upgrade, it becomes apparent that the operation does not verify whether the weak is locked or not. It just verifies whether the strong is zero or not. So, "locking the ability to upgrade weak pointers" sounds like the operation cannot succeed if weak is locked. IIUC, the actual intent should be that the success of the locking indicates that no other weak pointers exist, so it's impossible that there is a potential concurrent upgrade operation.

The suggested comment might be:

// the value usize::MAX acts as a sentinel for temporarily "locking" the
// weak count. This prevents `Arc::downgrade` from creating new Weak pointers
// while a check for uniqueness is in progress.
//
// This lock is essential for the correctness of `Arc::get_mut` and `Arc::is_unique`.
// By successfully acquiring the lock (which is only possible if the weak count is 1),
// we prove that no other `Weak` pointers exist at that moment. This guarantees
// that no concurrent `Weak::upgrade` can occur to violate the uniqueness of the
// mutable reference we are about to create.

Metadata

Metadata

Assignees

No one assigned

    Labels

    A-docsArea: Documentation for any part of the project, including the compiler, standard library, and toolsT-libsRelevant to the library team, which will review and decide on the PR/issue.

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions