Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

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

Closed
shahn opened this issue Dec 17, 2015 · 15 comments
Closed

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

shahn opened this issue Dec 17, 2015 · 15 comments
Labels
B-unstable Feature: Implemented in the nightly compiler and unstable. final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.

Comments

@shahn
Copy link
Contributor

shahn commented Dec 17, 2015

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
@Aatch Aatch added A-libs T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. labels Dec 17, 2015
@Aatch
Copy link
Contributor

Aatch commented Dec 17, 2015

/cc @rust-lang/libs

@sfackler
Copy link
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
Copy link
Member

huonw commented Dec 17, 2015

@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
Copy link
Contributor

Gankra commented Dec 17, 2015

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

@oli-obk
Copy link
Contributor

oli-obk commented Dec 17, 2015

Weak could simply implement Default...

@shahn
Copy link
Contributor Author

shahn commented Dec 17, 2015

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

@sfackler
Copy link
Member

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

@Gankra
Copy link
Contributor

Gankra commented Dec 18, 2015

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
Copy link
Contributor

Veedrac commented Dec 18, 2015

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
Copy link
Contributor Author

shahn commented Dec 18, 2015

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

shahn added a commit to shahn/rust that referenced this issue Dec 18, 2015
@shahn
Implement Weak::new_downgraded() (rust-lang#30425)
79d0235 
This adds a constructor for a Weak that can never be upgraded. These are
mostly useless, but for example are required when deserializing.
@apasel422
Copy link
Contributor

This was implemented in #30467.

@shahn
Copy link
Contributor Author

shahn commented Feb 6, 2016

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

@apasel422 apasel422 added the B-unstable Feature: Implemented in the nightly compiler and unstable. label Feb 6, 2016
@bluss bluss changed the title No way to construct std::rc::Weak without a strong reference Tracking issue for downgraded_weak (No way to construct std::rc::Weak without a strong reference) Apr 15, 2016
@alexcrichton
Copy link
Member

🔔 This issue is now entering its cycle-long final comment period for stabilization 🔔

The libs team is also considering adding a Default impl for Weak as well.

@alexcrichton alexcrichton added final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. and removed I-nominated labels Apr 29, 2016
@alexcrichton
Copy link
Member

The libs team discussed this issue during triage yesterday and the decision was to stabilize with the Default impl as well

@alexcrichton alexcrichton mentioned this issue May 17, 2016
Merged
@SimonSapin
Copy link
Contributor

Sounds OK if we make sure to explain in documentation that the created Weak<T> doesn’t point to a T value and can never be upgraded, and include an example where it can be useful to do this.

alexcrichton added a commit to alexcrichton/rust that referenced this issue May 24, 2016
@alexcrichton
std: Stabilize APIs for the 1.10 release
cae91d7 
This commit applies the FCP decisions made by the libs team for the 1.10 cycle,
including both new stabilizations and deprecations. Specifically, the list of
APIs is:

Stabilized:

* `os::windows::fs::OpenOptionsExt::access_mode`
* `os::windows::fs::OpenOptionsExt::share_mode`
* `os::windows::fs::OpenOptionsExt::custom_flags`
* `os::windows::fs::OpenOptionsExt::attributes`
* `os::windows::fs::OpenOptionsExt::security_qos_flags`
* `os::unix::fs::OpenOptionsExt::custom_flags`
* `sync::Weak::new`
* `Default for sync::Weak`
* `panic::set_hook`
* `panic::take_hook`
* `panic::PanicInfo`
* `panic::PanicInfo::payload`
* `panic::PanicInfo::location`
* `panic::Location`
* `panic::Location::file`
* `panic::Location::line`
* `ffi::CStr::from_bytes_with_nul`
* `ffi::CStr::from_bytes_with_nul_unchecked`
* `ffi::FromBytesWithNulError`
* `fs::Metadata::modified`
* `fs::Metadata::accessed`
* `fs::Metadata::created`
* `sync::atomic::Atomic{Usize,Isize,Bool,Ptr}::compare_exchange`
* `sync::atomic::Atomic{Usize,Isize,Bool,Ptr}::compare_exchange_weak`
* `collections::{btree,hash}_map::{Occupied,Vacant,}Entry::key`
* `os::unix::net::{UnixStream, UnixListener, UnixDatagram, SocketAddr}`
* `SocketAddr::is_unnamed`
* `SocketAddr::as_pathname`
* `UnixStream::connect`
* `UnixStream::pair`
* `UnixStream::try_clone`
* `UnixStream::local_addr`
* `UnixStream::peer_addr`
* `UnixStream::set_read_timeout`
* `UnixStream::set_write_timeout`
* `UnixStream::read_timeout`
* `UnixStream::write_Timeout`
* `UnixStream::set_nonblocking`
* `UnixStream::take_error`
* `UnixStream::shutdown`
* Read/Write/RawFd impls for `UnixStream`
* `UnixListener::bind`
* `UnixListener::accept`
* `UnixListener::try_clone`
* `UnixListener::local_addr`
* `UnixListener::set_nonblocking`
* `UnixListener::take_error`
* `UnixListener::incoming`
* RawFd impls for `UnixListener`
* `UnixDatagram::bind`
* `UnixDatagram::unbound`
* `UnixDatagram::pair`
* `UnixDatagram::connect`
* `UnixDatagram::try_clone`
* `UnixDatagram::local_addr`
* `UnixDatagram::peer_addr`
* `UnixDatagram::recv_from`
* `UnixDatagram::recv`
* `UnixDatagram::send_to`
* `UnixDatagram::send`
* `UnixDatagram::set_read_timeout`
* `UnixDatagram::set_write_timeout`
* `UnixDatagram::read_timeout`
* `UnixDatagram::write_timeout`
* `UnixDatagram::set_nonblocking`
* `UnixDatagram::take_error`
* `UnixDatagram::shutdown`
* RawFd impls for `UnixDatagram`
* `{BTree,Hash}Map::values_mut`
* `<[_]>::binary_search_by_key`

Deprecated:

* `StaticCondvar` - this, and all other static synchronization primitives
                    below, are usable today through the lazy-static crate on
                    stable Rust today. Additionally, we'd like the non-static
                    versions to be directly usable in a static context one day,
                    so they're unlikely to be the final forms of the APIs in any
                    case.
* `CONDVAR_INIT`
* `StaticMutex`
* `MUTEX_INIT`
* `StaticRwLock`
* `RWLOCK_INIT`
* `iter::Peekable::is_empty`

Closes rust-lang#27717
Closes rust-lang#27720
cc rust-lang#27784 (but encode methods still exist)
Closes rust-lang#30014
Closes rust-lang#30425
Closes rust-lang#30449
Closes rust-lang#31190
Closes rust-lang#31399
Closes rust-lang#31767
Closes rust-lang#32111
Closes rust-lang#32281
Closes rust-lang#32312
Closes rust-lang#32551
Closes rust-lang#33018
bors added a commit that referenced this issue May 26, 2016
@bors
Auto merge of #33699 - alexcrichton:stabilize-1.10, r=aturon
d5759a3 
std: Stabilize APIs for the 1.10 release

This commit applies the FCP decisions made by the libs team for the 1.10 cycle,
including both new stabilizations and deprecations. Specifically, the list of
APIs is:

Stabilized:

* `os::windows::fs::OpenOptionsExt::access_mode`
* `os::windows::fs::OpenOptionsExt::share_mode`
* `os::windows::fs::OpenOptionsExt::custom_flags`
* `os::windows::fs::OpenOptionsExt::attributes`
* `os::windows::fs::OpenOptionsExt::security_qos_flags`
* `os::unix::fs::OpenOptionsExt::custom_flags`
* `sync::Weak::new`
* `Default for sync::Weak`
* `panic::set_hook`
* `panic::take_hook`
* `panic::PanicInfo`
* `panic::PanicInfo::payload`
* `panic::PanicInfo::location`
* `panic::Location`
* `panic::Location::file`
* `panic::Location::line`
* `ffi::CStr::from_bytes_with_nul`
* `ffi::CStr::from_bytes_with_nul_unchecked`
* `ffi::FromBytesWithNulError`
* `fs::Metadata::modified`
* `fs::Metadata::accessed`
* `fs::Metadata::created`
* `sync::atomic::Atomic{Usize,Isize,Bool,Ptr}::compare_exchange`
* `sync::atomic::Atomic{Usize,Isize,Bool,Ptr}::compare_exchange_weak`
* `collections::{btree,hash}_map::{Occupied,Vacant,}Entry::key`
* `os::unix::net::{UnixStream, UnixListener, UnixDatagram, SocketAddr}`
* `SocketAddr::is_unnamed`
* `SocketAddr::as_pathname`
* `UnixStream::connect`
* `UnixStream::pair`
* `UnixStream::try_clone`
* `UnixStream::local_addr`
* `UnixStream::peer_addr`
* `UnixStream::set_read_timeout`
* `UnixStream::set_write_timeout`
* `UnixStream::read_timeout`
* `UnixStream::write_Timeout`
* `UnixStream::set_nonblocking`
* `UnixStream::take_error`
* `UnixStream::shutdown`
* Read/Write/RawFd impls for `UnixStream`
* `UnixListener::bind`
* `UnixListener::accept`
* `UnixListener::try_clone`
* `UnixListener::local_addr`
* `UnixListener::set_nonblocking`
* `UnixListener::take_error`
* `UnixListener::incoming`
* RawFd impls for `UnixListener`
* `UnixDatagram::bind`
* `UnixDatagram::unbound`
* `UnixDatagram::pair`
* `UnixDatagram::connect`
* `UnixDatagram::try_clone`
* `UnixDatagram::local_addr`
* `UnixDatagram::peer_addr`
* `UnixDatagram::recv_from`
* `UnixDatagram::recv`
* `UnixDatagram::send_to`
* `UnixDatagram::send`
* `UnixDatagram::set_read_timeout`
* `UnixDatagram::set_write_timeout`
* `UnixDatagram::read_timeout`
* `UnixDatagram::write_timeout`
* `UnixDatagram::set_nonblocking`
* `UnixDatagram::take_error`
* `UnixDatagram::shutdown`
* RawFd impls for `UnixDatagram`
* `{BTree,Hash}Map::values_mut`
* `<[_]>::binary_search_by_key`

Deprecated:

* `StaticCondvar` - this, and all other static synchronization primitives
                    below, are usable today through the lazy-static crate on
                    stable Rust today. Additionally, we'd like the non-static
                    versions to be directly usable in a static context one day,
                    so they're unlikely to be the final forms of the APIs in any
                    case.
* `CONDVAR_INIT`
* `StaticMutex`
* `MUTEX_INIT`
* `StaticRwLock`
* `RWLOCK_INIT`
* `iter::Peekable::is_empty`

Closes #27717
Closes #27720
Closes #30014
Closes #30425
Closes #30449
Closes #31190
Closes #31399
Closes #31767
Closes #32111
Closes #32281
Closes #32312
Closes #32551
Closes #33018
alexcrichton added a commit to alexcrichton/rust that referenced this issue May 26, 2016
@alexcrichton
std: Stabilize APIs for the 1.10 release
a2141e2 
This commit applies the FCP decisions made by the libs team for the 1.10 cycle,
including both new stabilizations and deprecations. Specifically, the list of
APIs is:

Stabilized:

* `os::windows::fs::OpenOptionsExt::access_mode`
* `os::windows::fs::OpenOptionsExt::share_mode`
* `os::windows::fs::OpenOptionsExt::custom_flags`
* `os::windows::fs::OpenOptionsExt::attributes`
* `os::windows::fs::OpenOptionsExt::security_qos_flags`
* `os::unix::fs::OpenOptionsExt::custom_flags`
* `sync::Weak::new`
* `Default for sync::Weak`
* `panic::set_hook`
* `panic::take_hook`
* `panic::PanicInfo`
* `panic::PanicInfo::payload`
* `panic::PanicInfo::location`
* `panic::Location`
* `panic::Location::file`
* `panic::Location::line`
* `ffi::CStr::from_bytes_with_nul`
* `ffi::CStr::from_bytes_with_nul_unchecked`
* `ffi::FromBytesWithNulError`
* `fs::Metadata::modified`
* `fs::Metadata::accessed`
* `fs::Metadata::created`
* `sync::atomic::Atomic{Usize,Isize,Bool,Ptr}::compare_exchange`
* `sync::atomic::Atomic{Usize,Isize,Bool,Ptr}::compare_exchange_weak`
* `collections::{btree,hash}_map::{Occupied,Vacant,}Entry::key`
* `os::unix::net::{UnixStream, UnixListener, UnixDatagram, SocketAddr}`
* `SocketAddr::is_unnamed`
* `SocketAddr::as_pathname`
* `UnixStream::connect`
* `UnixStream::pair`
* `UnixStream::try_clone`
* `UnixStream::local_addr`
* `UnixStream::peer_addr`
* `UnixStream::set_read_timeout`
* `UnixStream::set_write_timeout`
* `UnixStream::read_timeout`
* `UnixStream::write_Timeout`
* `UnixStream::set_nonblocking`
* `UnixStream::take_error`
* `UnixStream::shutdown`
* Read/Write/RawFd impls for `UnixStream`
* `UnixListener::bind`
* `UnixListener::accept`
* `UnixListener::try_clone`
* `UnixListener::local_addr`
* `UnixListener::set_nonblocking`
* `UnixListener::take_error`
* `UnixListener::incoming`
* RawFd impls for `UnixListener`
* `UnixDatagram::bind`
* `UnixDatagram::unbound`
* `UnixDatagram::pair`
* `UnixDatagram::connect`
* `UnixDatagram::try_clone`
* `UnixDatagram::local_addr`
* `UnixDatagram::peer_addr`
* `UnixDatagram::recv_from`
* `UnixDatagram::recv`
* `UnixDatagram::send_to`
* `UnixDatagram::send`
* `UnixDatagram::set_read_timeout`
* `UnixDatagram::set_write_timeout`
* `UnixDatagram::read_timeout`
* `UnixDatagram::write_timeout`
* `UnixDatagram::set_nonblocking`
* `UnixDatagram::take_error`
* `UnixDatagram::shutdown`
* RawFd impls for `UnixDatagram`
* `{BTree,Hash}Map::values_mut`
* `<[_]>::binary_search_by_key`

Deprecated:

* `StaticCondvar` - this, and all other static synchronization primitives
                    below, are usable today through the lazy-static crate on
                    stable Rust today. Additionally, we'd like the non-static
                    versions to be directly usable in a static context one day,
                    so they're unlikely to be the final forms of the APIs in any
                    case.
* `CONDVAR_INIT`
* `StaticMutex`
* `MUTEX_INIT`
* `StaticRwLock`
* `RWLOCK_INIT`
* `iter::Peekable::is_empty`

Closes rust-lang#27717
Closes rust-lang#27720
cc rust-lang#27784 (but encode methods still exist)
Closes rust-lang#30014
Closes rust-lang#30425
Closes rust-lang#30449
Closes rust-lang#31190
Closes rust-lang#31399
Closes rust-lang#31767
Closes rust-lang#32111
Closes rust-lang#32281
Closes rust-lang#32312
Closes rust-lang#32551
Closes rust-lang#33018
@oli-obk oli-obk mentioned this issue Jun 29, 2016
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
B-unstable Feature: Implemented in the nightly compiler and unstable. final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
10 participants
@alexcrichton @shahn @SimonSapin @oli-obk @Aatch @Gankra @huonw @sfackler @Veedrac @apasel422