Skip to content

Commit 987dc84

Browse files
committed
std: Fix a TLS destructor bug on OSX
TLS tests have been deadlocking on the OSX bots for quite some time now and this commit is the result of the investigation into what's going on. It turns out that a value in TLS which is being destroyed (e.g. the destructor is run) can be reset back to the initial state **while the destructor is running** if TLS is re-accessed. To fix this we stop calling drop_in_place on OSX and instead move the data to a temporary location on the stack.
1 parent fe0b5c0 commit 987dc84

File tree

2 files changed

+57
-2
lines changed

2 files changed

+57
-2
lines changed

src/libstd/thread/local.rs

+19-2
Original file line numberDiff line numberDiff line change
@@ -275,6 +275,7 @@ mod imp {
275275

276276
use cell::{Cell, UnsafeCell};
277277
use intrinsics;
278+
use ptr;
278279

279280
pub struct Key<T> {
280281
inner: UnsafeCell<Option<T>>,
@@ -327,7 +328,6 @@ mod imp {
327328
#[cfg(target_os = "linux")]
328329
unsafe fn register_dtor(t: *mut u8, dtor: unsafe extern fn(*mut u8)) {
329330
use mem;
330-
use ptr;
331331
use libc;
332332
use sys_common::thread_local as os;
333333

@@ -394,7 +394,24 @@ mod imp {
394394
// destructor as running for this thread so calls to `get` will return
395395
// `None`.
396396
(*ptr).dtor_running.set(true);
397-
intrinsics::drop_in_place((*ptr).inner.get());
397+
398+
// The OSX implementation of TLS apparently had an odd aspect to it
399+
// where the pointer we have may be overwritten while this destructor
400+
// is running. Specifically if a TLS destructor re-accesses TLS it may
401+
// trigger a re-initialization of all TLS variables, paving over at
402+
// least some destroyed ones with initial values.
403+
//
404+
// This means that if we drop a TLS value in place on OSX that we could
405+
// revert the value to its original state halfway through the
406+
// destructor, which would be bad!
407+
//
408+
// Hence, we use `ptr::read` on OSX (to move to a "safe" location)
409+
// instead of drop_in_place.
410+
if cfg!(target_os = "macos") {
411+
ptr::read((*ptr).inner.get());
412+
} else {
413+
intrinsics::drop_in_place((*ptr).inner.get());
414+
}
398415
}
399416
}
400417

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
// Copyright 2015 The Rust Project Developers. See the COPYRIGHT
2+
// file at the top-level directory of this distribution and at
3+
// http://rust-lang.org/COPYRIGHT.
4+
//
5+
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
6+
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
7+
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
8+
// option. This file may not be copied, modified, or distributed
9+
// except according to those terms.
10+
11+
thread_local!(static FOO: Foo = Foo);
12+
thread_local!(static BAR: Bar = Bar(1));
13+
thread_local!(static BAZ: Baz = Baz);
14+
15+
struct Foo;
16+
struct Bar(i32);
17+
struct Baz;
18+
19+
impl Drop for Foo {
20+
fn drop(&mut self) {
21+
BAR.with(|_| {});
22+
}
23+
}
24+
25+
impl Drop for Bar {
26+
fn drop(&mut self) {
27+
assert_eq!(self.0, 1);
28+
self.0 = 2;
29+
BAZ.with(|_| {});
30+
assert_eq!(self.0, 2);
31+
}
32+
}
33+
34+
fn main() {
35+
std::thread::spawn(|| {
36+
FOO.with(|_| {});
37+
}).join().unwrap();
38+
}

0 commit comments

Comments
 (0)