Skip to content

Tracking issue for downgraded_weak (No way to construct std::rc::Weak without a strong reference) #30425

Closed
@shahn

Description

@shahn
Contributor

When deserializing Weak, there's currently no clean way to handle the case that there's no strong reference anymore. Serializing doesn't have the same issue. There seem to be two solutions to this, either provide an explicit constructor that returns a Weak without using a strong reference to create it, or implementing Default for it that does the same. I prefer the former, as Default might be a bit surprising.

This would be a simple, non-backwards-incompatible change. Possible constructor name bikeshedding: Weak::new_downgraded()

Unstable APIs

  • Weak::new

Activity

added
T-libs-apiRelevant to the library API team, which will review and decide on the PR/issue.
on Dec 17, 2015
Aatch

Aatch commented on Dec 17, 2015

@Aatch
Contributor

/cc @rust-lang/libs

sfackler

sfackler commented on Dec 17, 2015

@sfackler
Member

I feel like I'm missing a bit of context here - wouldn't the stored object immediately be freed since the strong reference count will be zero?

huonw

huonw commented on Dec 17, 2015

@huonw
Member

@sfackler, yeah, the Weak<T> never actually contains a T so it's always non-upgradable, but it allows deserialising things that happen to contain a Weak (where it isn't unexpected for the Weak to be invalid).

Gankra

Gankra commented on Dec 17, 2015

@Gankra
Contributor

Seems easy enough to add. Basically Option::None.

oli-obk

oli-obk commented on Dec 17, 2015

@oli-obk
Contributor

Weak could simply implement Default...

shahn

shahn commented on Dec 17, 2015

@shahn
ContributorAuthor

@oli-obk It could, but wouldn't it be a bit surprising that the Default for Weak is an unusable Weak?

sfackler

sfackler commented on Dec 17, 2015

@sfackler
Member

Ah, got it. Weak::new_empty() (or whatever name) seems reasonable enough.

Gankra

Gankra commented on Dec 18, 2015

@Gankra
Contributor

Just to be clear: an empty Weak would still allocate memory. We're all in agreement that this is the desired semantic? The alternative would be adding a sentinel state to every weak pointer that needs to be checked on every access.

Veedrac

Veedrac commented on Dec 18, 2015

@Veedrac
Contributor

If you care about allocating memory for empty Weaks, you could make them point to some thread-local instead. Just increment the thread-local's weak count on construction and make sure it starts with a count of 1 so it never gets deallocated.

It sounds like a needless optimization for a rare case, and the aliasing is probably undefined behaviour without care, but it's workable if you really want it.

shahn

shahn commented on Dec 18, 2015

@shahn
ContributorAuthor

Yes, it's fine to allocate the memory (you even want to do it to match the pre-serialization state). Having a constructor called new_* should make that clear, imo

apasel422

apasel422 commented on Feb 6, 2016

@apasel422
Contributor

This was implemented in #30467.

shahn

shahn commented on Feb 6, 2016

@shahn
ContributorAuthor

I think this issue needs to stay open until a stabilization decision was made

29 remaining items

Loading
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

    B-unstableBlocker: Implemented in the nightly compiler and unstable.T-libs-apiRelevant to the library API team, which will review and decide on the PR/issue.final-comment-periodIn the final comment period and will be merged soon unless new substantive objections are raised.

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

      Development

      No branches or pull requests

        Participants

        @alexcrichton@shahn@SimonSapin@oli-obk@Aatch

        Issue actions

          Tracking issue for `downgraded_weak` (No way to construct std::rc::Weak without a strong reference) · Issue #30425 · rust-lang/rust