Skip to content

Tracking Issue for {Arc, Rc}::into_inner #106894

Closed

Description

@steffahn

Feature gates: #![feature(arc_into_inner)], #![feature(rc_into_inner)]

This is a tracking issue for the Arc::into_inner method, and (for API-consistency) also an Rc::into_inner analogue.

The function Arc::into_inner(x) serves as an atomic race-condition-free alternative to Arc::try_unwrap(x).ok(). See the ACP for more details.

Public API

// alloc::sync

impl<T> Arc<T> {
    /// Returns the inner value, if the `Arc` has exactly one strong reference.
    /// Otherwise, `None` is returned and the `Arc` is dropped.
    pub fn into_inner(this: Arc<T>) -> Option<T>;
}

and the extension to Rc

// alloc::rc

impl<T> Rc<T> {
    /// Returns the inner value, if the `Rc` has exactly one strong reference.
    /// Otherwise, `None` is returned and the `Rc` is dropped.
    pub fn into_inner(this: Rc<T>) -> Option<T>;
}

Steps / History

Unresolved Questions

  • None yet.

Footnotes

  1. https://std-dev-guide.rust-lang.org/feature-lifecycle/stabilization.html ↩

Activity

  1. added
    C-tracking-issueCategory: An issue tracking the progress of sth. like the implementation of an RFC
    T-libs-api[DEPRECATED; DO NOT USE]
    on Jan 15, 2023
  2. added a commit that references this issue on Jan 23, 2023
  3. ryoqun commented on Mar 1, 2023

    @ryoqun
    Contributor

    Hey, I've stumbled on this. I wonder how soon this can be stabilized? My usecase is complex. but in a nut shell, I want this to conditionally and cleanly restart bunch of interconnected threads of mixed roles, all sharing Arc<GlobalContext>, depending on which thread did actually dropped the GlobalContext.

    if sharing helps, i can write the usecase in detail...

    Of course, I can resort to sync::Weak or tls. but it incurs overhead and/or further complexities. or mem::transmute like this to forcibly bring ::into_inner into stable rustc: https://rust.godbolt.org/z/r5rbsYYcT, in which i have to use unsafe... Also, due to already-existing wide-spread use of Arc<GlobalContext> in our codebase, switching to different Arc impl altogether isn't viable as well. That precondition also precludes more saner cleaning code arrangement in the first place. ;)

  4. joshtriplett commented on Mar 11, 2023

    @joshtriplett
    Member

    Submitted the Rc::into_inner implementation as #109026

  5. joshtriplett commented on Mar 11, 2023

    @joshtriplett
    Member

    I think Arc::into_inner makes sense to stabilize at this point. And Rc::into_inner is trivial.

    @rust-lang/libs-api, shall we stabilize these?

    @rfcbot merge

  6. rfcbot commented on Mar 11, 2023

    @rfcbot

    Team member @joshtriplett has proposed to merge this. The next step is review by the rest of the tagged team members:

    No concerns currently listed.

    Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

    See this document for info about what commands tagged team members can give me.

  7. added
    proposed-final-comment-periodProposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off.
    disposition-mergeThis issue / PR is in PFCP or FCP with a disposition to merge it.
    final-comment-periodIn the final comment period and will be merged soon unless new substantive objections are raised.
    on Mar 11, 2023
  8. rfcbot commented on Mar 12, 2023

    @rfcbot

    🔔 This is now entering its final comment period, as per the review above. 🔔

  9. removed
    proposed-final-comment-periodProposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off.
    on Mar 12, 2023
  10. added
    to-announceAnnounce this issue on triage meeting
    and removed
    final-comment-periodIn the final comment period and will be merged soon unless new substantive objections are raised.
    on Mar 22, 2023
  11. 6 remaining items

  12. thynson commented on Jan 23, 2024

    @thynson

    Probably I'm too late here, but I think the Arc::<T>::into_inner should returns Result<T, Self>, instead of Option<T>.
    Unlike Rc, checking Arc::strong_count(arc) == 1 does not guarantee that the subsequence call to Arc::<T>::into_inner will no fail. And when it happens, the caller lost an owning Arc, making it impossible to recover from such a situation.

    fn unbox_from_arc(arc: Arc<T>) -> T {
      if Arc::strong_count(&arc) == 1 {
        // RACE CONDITION: another thread could upgrade a corresponding Weak<T> at this moment
        return Arc::into_inner(arc)
        // We have no chance to recover from the failure, since `arc` was consumed by`Arc::into_inner`
        // the only thing we could do is to panic
        .unwrap();    
      }
    }

    Since Arc::into_inner has been stabilized, a new interface Arc::try_into_inner::<T>(x: Arc<T>) -> Result<T, Arc<T>> could be proposed. It should check the reference count is 1 and unwrap it atomically, or returns the Arc itself when the condition was not met, so the caller still have an owning Arc.

    In theory, the above issue is not applicable for Rc, but Rc::try_into_inner::<T>(x: Rc<T>) -> Result<T, Rc<T>> could also be proposed for consistency.

  13. steffahn commented on Jan 23, 2024

    @steffahn
    MemberAuthor

    @thynson Feel free to open an issue if you have concrete ideas how the documentation of Arc::into_inner can be improved to convince users to actually look at the Arc::try_unwrap function. Of course, it's already mentioned there multiple times (it's also mentioned on this tracking issue) but apparently not in a way that convinced you to actually look at that method.

    Maybe also a doc alias “try_into_inner” for try_unwrap to further help discoverability 🤔 (though I'm not sure off the top of my head, how well those work for associated items; and also that would assume people use the search bar, anyways)

  14. thynson commented on Jan 23, 2024

    @thynson

    Umm, it's my bad that Arc::try_unwrap already mentioned in the doc of Arc::into_inner, which is exactly what I'm looking for, through the doc make me feel it's discouraged to be used

        /// ...
        ///
        /// The similar expression `Arc::try_unwrap(this).ok()` does not
        /// offer such a guarantee. See the last example below
        /// and the documentation of [`Arc::try_unwrap`].
        ///
        /// ...
    

    Actually they're designed for different scenarios. Probably better to document it like:

        /// ...
        ///
        /// `Arc::try_unwrap` offers similar functionality but it's designed 
        /// for difference scenarios, while this function is suitable for dropping
        /// a clone of an `Arc`, obtaining the wrapped `T` when the last clone of an
        ///  `Arc` was dropped.
        ///
        /// ...
    
  15. steffahn commented on Jan 23, 2024

    @steffahn
    MemberAuthor

    Hmm, no try_unwrap is only meant to be discouraged in the case where you ignore the Arc<T> you get back in the Err case. That’s clearly documented, too… but it’s describing the details of when and how to use try_unwrap, and hence not documented in into_inner’s documentation.

    On the other hand, I wasn’t aware that someone could try to combine into_inner with a Arc::strong_count(…) == 1 check.

    Anyways… maybe a sentence that introduces Arc::try_unwrap before immediately calling out an expression using it could be sufficiently useful, after all.

    If Arc::into_inner is called on every clone of this Arc, it is guaranteed that exactly one of the calls returns the inner value. This means in particular that the inner value is not dropped.

    Arc::try_unwrap offers similar functionality to this function, and might seem simply like a more general/powerful API than Arc::into_inner; but it has in fact different use-cases. It you did use it to replace Arc::into_inner, such as with the expression Arc::try_unwrap(this).ok(), then you would not get the guarantee described in the previous paragraph. For more information, see the last example below and read the documentation of Arc::try_unwrap.

    ^^^ Maybe like this?

  16. thynson commented on Jan 23, 2024

    @thynson

    It you did use it to replace Arc::into_inner, ...

    If I understand it right, it looks like a typos of

    If you did use it to replace Arc::into_inner, ...

    The modified version should be clear enough for me.
    And thanks for your quick response. 🥇

  17. steffahn commented on Jan 23, 2024

    @steffahn
    MemberAuthor

    @thynson I’ve changed the wording a bit more, feel free to take a look #120266

  18. added 5 commits that reference this issue on Jan 27, 2024
  19. added a commit that references this issue on Jan 29, 2024
  20. added a commit that references this issue on Jan 29, 2024
  21. added a commit that references this issue on Jan 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    C-tracking-issueCategory: An issue tracking the progress of sth. like the implementation of an RFCT-libs-api[DEPRECATED; DO NOT USE]disposition-mergeThis issue / PR is in PFCP or FCP with a disposition to merge it.finished-final-comment-periodThe final comment period is finished for this PR / Issue.

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions