Skip to content

Commit 64e81e5

Browse files
authored
Rollup merge of #146799 - cuviper:dangling-count-latch, r=lcnr
Fix a dangling reference in `rustc_thread_pool` This diverged from `rayon` in #142384, where a cleanup commit turned the matched `worker_index` into a reference, which is read _after_ the `set` that may kill it. I've moved that read beforehand, and I hope the new comments will emphasize the subtlety of this unsafe code. Hopefully fixes #146677.
2 parents 8f11c4d + 389a502 commit 64e81e5

File tree

1 file changed

+6
-2
lines changed

1 file changed

+6
-2
lines changed

compiler/rustc_thread_pool/src/latch.rs

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -388,13 +388,17 @@ impl Latch for CountLatch {
388388
#[inline]
389389
unsafe fn set(this: *const Self) {
390390
if unsafe { (*this).counter.fetch_sub(1, Ordering::SeqCst) == 1 } {
391-
// NOTE: Once we call `set` on the internal `latch`,
391+
// SAFETY: Once we call `set` on the internal `latch`,
392392
// the target may proceed and invalidate `this`!
393393
match unsafe { &(*this).kind } {
394394
CountLatchKind::Stealing { latch, registry, worker_index } => {
395395
let registry = Arc::clone(registry);
396+
let worker_index = *worker_index;
397+
// SAFETY: We don't use any references from `this` after this call.
396398
if unsafe { CoreLatch::set(latch) } {
397-
registry.notify_worker_latch_is_set(*worker_index);
399+
// We **must not** access any part of `this` anymore, which
400+
// is why we read and shadowed these fields beforehand.
401+
registry.notify_worker_latch_is_set(worker_index);
398402
}
399403
}
400404
CountLatchKind::Blocking { latch } => unsafe { LockLatch::set(latch) },

0 commit comments

Comments
 (0)