Skip to content

Commit bcb4feb

Browse files
committed
Use an aligned dangling pointer in Weak::new, rather than address 1
1 parent 64f8ae0 commit bcb4feb

File tree

1 file changed

+29
-21
lines changed

1 file changed

+29
-21
lines changed

src/liballoc/sync.rs

+29-21
Original file line numberDiff line numberDiff line change
@@ -43,9 +43,6 @@ use vec::Vec;
4343
/// necessarily) at _exactly_ `MAX_REFCOUNT + 1` references.
4444
const MAX_REFCOUNT: usize = (isize::MAX) as usize;
4545

46-
/// A sentinel value that is used for the pointer of `Weak::new()`.
47-
const WEAK_EMPTY: usize = 1;
48-
4946
/// A thread-safe reference-counting pointer. 'Arc' stands for 'Atomically
5047
/// Reference Counted'.
5148
///
@@ -239,9 +236,9 @@ impl<T: ?Sized + Unsize<U>, U: ?Sized> CoerceUnsized<Arc<U>> for Arc<T> {}
239236
#[stable(feature = "arc_weak", since = "1.4.0")]
240237
pub struct Weak<T: ?Sized> {
241238
// This is a `NonNull` to allow optimizing the size of this type in enums,
242-
// but it is actually not truly "non-null". A `Weak::new()` will set this
243-
// to a sentinel value, instead of needing to allocate some space in the
244-
// heap.
239+
// but it is not necessarily a valid pointer.
240+
// `Weak::new` sets this to a dangling pointer so that it doesn’t need
241+
// to allocate space on the heap.
245242
ptr: NonNull<ArcInner<T>>,
246243
}
247244

@@ -1034,14 +1031,18 @@ impl<T> Weak<T> {
10341031
/// ```
10351032
#[stable(feature = "downgraded_weak", since = "1.10.0")]
10361033
pub fn new() -> Weak<T> {
1037-
unsafe {
1038-
Weak {
1039-
ptr: NonNull::new_unchecked(WEAK_EMPTY as *mut _),
1040-
}
1034+
Weak {
1035+
ptr: NonNull::dangling(),
10411036
}
10421037
}
10431038
}
10441039

1040+
fn is_dangling<T: ?Sized>(ptr: NonNull<T>) -> bool {
1041+
let address = ptr.as_ptr() as *mut () as usize;
1042+
let align = align_of_val(unsafe { ptr.as_ref() });
1043+
address == align
1044+
}
1045+
10451046
impl<T: ?Sized> Weak<T> {
10461047
/// Attempts to upgrade the `Weak` pointer to an [`Arc`], extending
10471048
/// the lifetime of the value if successful.
@@ -1073,11 +1074,7 @@ impl<T: ?Sized> Weak<T> {
10731074
pub fn upgrade(&self) -> Option<Arc<T>> {
10741075
// We use a CAS loop to increment the strong count instead of a
10751076
// fetch_add because once the count hits 0 it must never be above 0.
1076-
let inner = if self.ptr.as_ptr() as *const u8 as usize == WEAK_EMPTY {
1077-
return None;
1078-
} else {
1079-
unsafe { self.ptr.as_ref() }
1080-
};
1077+
let inner = self.inner()?;
10811078

10821079
// Relaxed load because any write of 0 that we can observe
10831080
// leaves the field in a permanently zero state (so a
@@ -1108,6 +1105,17 @@ impl<T: ?Sized> Weak<T> {
11081105
}
11091106
}
11101107
}
1108+
1109+
/// Return `None` when the pointer is dangling and there is no allocated `ArcInner`,
1110+
/// i.e. this `Weak` was created by `Weak::new`
1111+
#[inline]
1112+
fn inner(&self) -> Option<&ArcInner<T>> {
1113+
if is_dangling(self.ptr) {
1114+
None
1115+
} else {
1116+
Some(unsafe { self.ptr.as_ref() })
1117+
}
1118+
}
11111119
}
11121120

11131121
#[stable(feature = "arc_weak", since = "1.4.0")]
@@ -1125,10 +1133,10 @@ impl<T: ?Sized> Clone for Weak<T> {
11251133
/// ```
11261134
#[inline]
11271135
fn clone(&self) -> Weak<T> {
1128-
let inner = if self.ptr.as_ptr() as *const u8 as usize == WEAK_EMPTY {
1129-
return Weak { ptr: self.ptr };
1136+
let inner = if let Some(inner) = self.inner() {
1137+
inner
11301138
} else {
1131-
unsafe { self.ptr.as_ref() }
1139+
return Weak { ptr: self.ptr };
11321140
};
11331141
// See comments in Arc::clone() for why this is relaxed. This can use a
11341142
// fetch_add (ignoring the lock) because the weak count is only locked
@@ -1203,10 +1211,10 @@ impl<T: ?Sized> Drop for Weak<T> {
12031211
// weak count can only be locked if there was precisely one weak ref,
12041212
// meaning that drop could only subsequently run ON that remaining weak
12051213
// ref, which can only happen after the lock is released.
1206-
let inner = if self.ptr.as_ptr() as *const u8 as usize == WEAK_EMPTY {
1207-
return;
1214+
let inner = if let Some(inner) = self.inner() {
1215+
inner
12081216
} else {
1209-
unsafe { self.ptr.as_ref() }
1217+
return
12101218
};
12111219

12121220
if inner.weak.fetch_sub(1, Release) == 1 {

0 commit comments

Comments
 (0)