Skip to content

Commit e749ba2

Browse files
Rollup merge of #98364 - RalfJung:arc-clone, r=Mark-Simulacrum
clarify Arc::clone overflow check comment I had to read this twice to realize that this is explaining that the code is technically unsound, so move that into a dedicated paragraph and make the wording a bit more explicit.
2 parents cc45ad5 + 46b2454 commit e749ba2

File tree

1 file changed

+9
-8
lines changed

1 file changed

+9
-8
lines changed

library/alloc/src/sync.rs

+9-8
Original file line numberDiff line numberDiff line change
@@ -1355,15 +1355,16 @@ impl<T: ?Sized> Clone for Arc<T> {
13551355
// [1]: (www.boost.org/doc/libs/1_55_0/doc/html/atomic/usage_examples.html)
13561356
let old_size = self.inner().strong.fetch_add(1, Relaxed);
13571357

1358-
// However we need to guard against massive refcounts in case someone
1359-
// is `mem::forget`ing Arcs. If we don't do this the count can overflow
1360-
// and users will use-after free. We racily saturate to `isize::MAX` on
1361-
// the assumption that there aren't ~2 billion threads incrementing
1362-
// the reference count at once. This branch will never be taken in
1363-
// any realistic program.
1358+
// However we need to guard against massive refcounts in case someone is `mem::forget`ing
1359+
// Arcs. If we don't do this the count can overflow and users will use-after free. This
1360+
// branch will never be taken in any realistic program. We abort because such a program is
1361+
// incredibly degenerate, and we don't care to support it.
13641362
//
1365-
// We abort because such a program is incredibly degenerate, and we
1366-
// don't care to support it.
1363+
// This check is not 100% water-proof: we error when the refcount grows beyond `isize::MAX`.
1364+
// But we do that check *after* having done the increment, so there is a chance here that
1365+
// the worst already happened and we actually do overflow the `usize` counter. However, that
1366+
// requires the counter to grow from `isize::MAX` to `usize::MAX` between the increment
1367+
// above and the `abort` below, which seems exceedingly unlikely.
13671368
if old_size > MAX_REFCOUNT {
13681369
abort();
13691370
}

0 commit comments

Comments
 (0)