Skip to content

Commit 908017f

Browse files
Fix data race in thread::scope()
See # 98498 for more info
1 parent 9cf699d commit 908017f

File tree

1 file changed

+19
-4
lines changed

1 file changed

+19
-4
lines changed

library/std/src/thread/scoped.rs

+19-4
Original file line numberDiff line numberDiff line change
@@ -35,10 +35,13 @@ pub struct Scope<'scope, 'env: 'scope> {
3535
#[stable(feature = "scoped_threads", since = "1.63.0")]
3636
pub struct ScopedJoinHandle<'scope, T>(JoinInner<'scope, T>);
3737

38+
// Note: all of `ScopeData` fields must be interiorly mutable since
39+
// it may be deallocated in the middle of a `&self` method:
40+
// see `decrement_num_running_threads` below for more info.
3841
pub(super) struct ScopeData {
3942
num_running_threads: AtomicUsize,
4043
a_thread_panicked: AtomicBool,
41-
main_thread: Thread,
44+
main_thread: UnsafeCell<Thread>,
4245
}
4346

4447
impl ScopeData {
@@ -51,12 +54,24 @@ impl ScopeData {
5154
panic!("too many running threads in thread scope");
5255
}
5356
}
57+
fn main_thread(&self) -> &Thread {
58+
unsafe { &*self.main_thread.get() }
59+
}
5460
pub(super) fn decrement_num_running_threads(&self, panic: bool) {
5561
if panic {
5662
self.a_thread_panicked.store(true, Ordering::Relaxed);
5763
}
64+
let main_thread = self.main_thread().clone();
5865
if self.num_running_threads.fetch_sub(1, Ordering::Release) == 1 {
59-
self.main_thread.unpark();
66+
// By now, `num_running_threads` is `0`, so when `scope()` in the main thread
67+
// is unparked, it will complete its business and deallocate `*self`!
68+
// Two things to look after:
69+
// - it can spuriously unpark / wake up, **so `self` can no longer be used**, even
70+
// before we, ourselves, unpark. Hence why we've cloned the `main_thread`'s handle.
71+
// - no matter how it unparks, `*self` may be deallocated before this function
72+
// returns, so all of `*self` data, **including `main_thread`**, must be interiorly
73+
// mutable. See https://github.com/rust-lang/rust/issues/55005 for more info.
74+
main_thread.unpark();
6075
}
6176
}
6277
}
@@ -133,7 +148,7 @@ where
133148
let scope = Scope {
134149
data: ScopeData {
135150
num_running_threads: AtomicUsize::new(0),
136-
main_thread: current(),
151+
main_thread: current().into(),
137152
a_thread_panicked: AtomicBool::new(false),
138153
},
139154
env: PhantomData,
@@ -328,7 +343,7 @@ impl fmt::Debug for Scope<'_, '_> {
328343
f.debug_struct("Scope")
329344
.field("num_running_threads", &self.data.num_running_threads.load(Ordering::Relaxed))
330345
.field("a_thread_panicked", &self.data.a_thread_panicked.load(Ordering::Relaxed))
331-
.field("main_thread", &self.data.main_thread)
346+
.field("main_thread", self.data.main_thread())
332347
.finish_non_exhaustive()
333348
}
334349
}

0 commit comments

Comments
 (0)