Skip to content

Commit

Permalink
Rollup merge of rust-lang#85535 - dtolnay:weakdangle, r=kennytm
Browse files Browse the repository at this point in the history
Weak's type parameter may dangle on drop

Way back in rust-lang@34076bc, #\[may_dangle\] was added to Rc\<T\> and Arc\<T\>'s Drop impls. That appears to have been because a test added in rust-lang#28929 used Arc and Rc with dangling references at drop time. However, Weak was not covered by that test, and therefore no #\[may_dangle\] was forced to be added at the time.

As far as dropping, Weak has *even less need* to interact with the T than Rc and Arc do. Roughly speaking #\[may_dangle\] describes generic parameters that the outer type's Drop impl does not interact with except by possibly dropping them; no other interaction (such as trait method calls on the generic type) is permissible. It's clear this applies to Rc's and Arc's drop impl, which sometimes drop T but otherwise do not interact with one. It applies *even more* to Weak. Dropping a Weak cannot ever cause T's drop impl to run. Either there are strong references still in existence, in which case better not drop the T. Or there are no strong references still in existence, in which case the T would already have been dropped previously by the drop of the last strong count.
  • Loading branch information
JohnTitor authored May 25, 2021
2 parents c8a9c87 + 23a4050 commit 1ee6a4e
Show file tree
Hide file tree
Showing 4 changed files with 32 additions and 2 deletions.
2 changes: 1 addition & 1 deletion library/alloc/src/rc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2303,7 +2303,7 @@ impl<T: ?Sized> Weak<T> {
}

#[stable(feature = "rc_weak", since = "1.4.0")]
impl<T: ?Sized> Drop for Weak<T> {
unsafe impl<#[may_dangle] T: ?Sized> Drop for Weak<T> {
/// Drops the `Weak` pointer.
///
/// # Examples
Expand Down
2 changes: 1 addition & 1 deletion library/alloc/src/sync.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2015,7 +2015,7 @@ impl<T> Default for Weak<T> {
}

#[stable(feature = "arc_weak", since = "1.4.0")]
impl<T: ?Sized> Drop for Weak<T> {
unsafe impl<#[may_dangle] T: ?Sized> Drop for Weak<T> {
/// Drops the `Weak` pointer.
///
/// # Examples
Expand Down
15 changes: 15 additions & 0 deletions library/alloc/tests/arc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -195,3 +195,18 @@ fn shared_from_iter_trustedlen_no_fuse() {
assert_trusted_len(&iter);
assert_eq!(&[Box::new(42), Box::new(24)], &*iter.collect::<Rc<[_]>>());
}

#[test]
fn weak_may_dangle() {
fn hmm<'a>(val: &'a mut Weak<&'a str>) -> Weak<&'a str> {
val.clone()
}

// Without #[may_dangle] we get:
let mut val = Weak::new();
hmm(&mut val);
// ~~~~~~~~ borrowed value does not live long enough
//
// `val` dropped here while still borrowed
// borrow might be used here, when `val` is dropped and runs the `Drop` code for type `std::sync::Weak`
}
15 changes: 15 additions & 0 deletions library/alloc/tests/rc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -191,3 +191,18 @@ fn shared_from_iter_trustedlen_no_fuse() {
assert_trusted_len(&iter);
assert_eq!(&[Box::new(42), Box::new(24)], &*iter.collect::<Rc<[_]>>());
}

#[test]
fn weak_may_dangle() {
fn hmm<'a>(val: &'a mut Weak<&'a str>) -> Weak<&'a str> {
val.clone()
}

// Without #[may_dangle] we get:
let mut val = Weak::new();
hmm(&mut val);
// ~~~~~~~~ borrowed value does not live long enough
//
// `val` dropped here while still borrowed
// borrow might be used here, when `val` is dropped and runs the `Drop` code for type `std::rc::Weak`
}

0 comments on commit 1ee6a4e

Please sign in to comment.