From 987dc84b521a9be98099b504aa91b7baa9d22409 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Fri, 10 Jul 2015 17:46:20 -0700 Subject: [PATCH] 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. --- src/libstd/thread/local.rs | 21 ++++++++++-- src/test/run-pass/down-with-thread-dtors.rs | 38 +++++++++++++++++++++ 2 files changed, 57 insertions(+), 2 deletions(-) create mode 100644 src/test/run-pass/down-with-thread-dtors.rs diff --git a/src/libstd/thread/local.rs b/src/libstd/thread/local.rs index 60563340d10..e2873601a7b 100644 --- a/src/libstd/thread/local.rs +++ b/src/libstd/thread/local.rs @@ -275,6 +275,7 @@ mod imp { use cell::{Cell, UnsafeCell}; use intrinsics; + use ptr; pub struct Key { inner: UnsafeCell>, @@ -327,7 +328,6 @@ unsafe fn register_dtor(&self) { #[cfg(target_os = "linux")] unsafe fn register_dtor(t: *mut u8, dtor: unsafe extern fn(*mut u8)) { use mem; - use ptr; use libc; use sys_common::thread_local as os; @@ -394,7 +394,24 @@ fn _tlv_atexit(dtor: unsafe extern fn(*mut u8), // destructor as running for this thread so calls to `get` will return // `None`. (*ptr).dtor_running.set(true); - intrinsics::drop_in_place((*ptr).inner.get()); + + // The OSX implementation of TLS apparently had an odd aspect to it + // where the pointer we have may be overwritten while this destructor + // is running. Specifically if a TLS destructor re-accesses TLS it may + // trigger a re-initialization of all TLS variables, paving over at + // least some destroyed ones with initial values. + // + // This means that if we drop a TLS value in place on OSX that we could + // revert the value to its original state halfway through the + // destructor, which would be bad! + // + // Hence, we use `ptr::read` on OSX (to move to a "safe" location) + // instead of drop_in_place. + if cfg!(target_os = "macos") { + ptr::read((*ptr).inner.get()); + } else { + intrinsics::drop_in_place((*ptr).inner.get()); + } } } diff --git a/src/test/run-pass/down-with-thread-dtors.rs b/src/test/run-pass/down-with-thread-dtors.rs new file mode 100644 index 00000000000..ee835785cbe --- /dev/null +++ b/src/test/run-pass/down-with-thread-dtors.rs @@ -0,0 +1,38 @@ +// Copyright 2015 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +thread_local!(static FOO: Foo = Foo); +thread_local!(static BAR: Bar = Bar(1)); +thread_local!(static BAZ: Baz = Baz); + +struct Foo; +struct Bar(i32); +struct Baz; + +impl Drop for Foo { + fn drop(&mut self) { + BAR.with(|_| {}); + } +} + +impl Drop for Bar { + fn drop(&mut self) { + assert_eq!(self.0, 1); + self.0 = 2; + BAZ.with(|_| {}); + assert_eq!(self.0, 2); + } +} + +fn main() { + std::thread::spawn(|| { + FOO.with(|_| {}); + }).join().unwrap(); +} -- 2.44.0