std: Use mem::replace in TLS initialization

Due to #30228 it's not currently sound to do `*ptr = Some(value)`, so instead
use `mem::replace` which fixes the soundness hole for now.

2 files changed, 67 insertions, 1 deletions

diff --git a/src/libstd/thread/local.rs b/src/libstd/thread/local.rs
index 119429cc584..870247f7e82 100644
--- a/src/libstd/thread/local.rs
+++ b/src/libstd/thread/local.rs
@@ -13,6 +13,7 @@
 #![unstable(feature = "thread_local_internals", issue = "0")]
 
 use cell::UnsafeCell;
+use mem;
 
 // Sure wish we had macro hygiene, no?
 #[doc(hidden)]
@@ -226,7 +227,21 @@ impl<T: 'static> LocalKey<T> {
         // just in case initialization fails.
         let value = (self.init)();
         let ptr = slot.get();
-        *ptr = Some(value);
+
+        // note that this can in theory just be `*ptr = Some(value)`, but due to
+        // the compiler will currently codegen that pattern with something like:
+        //
+        //      ptr::drop_in_place(ptr)
+        //      ptr::write(ptr, Some(value))
+        //
+        // Due to this pattern it's possible for the destructor of the value in
+        // `ptr` (e.g. if this is being recursively initialized) to re-access
+        // TLS, in which case there will be a `&` and `&mut` pointer to the same
+        // value (an aliasing violation). To avoid setting the "I'm running a
+        // destructor" flag we just use `mem::replace` which should sequence the
+        // operations a little differently and make this safe to call.
+        mem::replace(&mut *ptr, Some(value));
+
         (*ptr).as_ref().unwrap()
     }
 
diff --git a/src/test/run-pass/tls-init-on-init.rs b/src/test/run-pass/tls-init-on-init.rs
new file mode 100644
index 00000000000..195b814492a
--- /dev/null
+++ b/src/test/run-pass/tls-init-on-init.rs
@@ -0,0 +1,51 @@
+// 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 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+#![feature(thread_local_state)]
+
+use std::thread::{self, LocalKeyState};
+use std::sync::atomic::{AtomicUsize, Ordering, ATOMIC_USIZE_INIT};
+
+struct Foo { cnt: usize }
+
+thread_local!(static FOO: Foo = Foo::init());
+
+static CNT: AtomicUsize = ATOMIC_USIZE_INIT;
+
+impl Foo {
+    fn init() -> Foo {
+        let cnt = CNT.fetch_add(1, Ordering::SeqCst);
+        if cnt == 0 {
+            FOO.with(|_| {});
+        }
+        Foo { cnt: cnt }
+    }
+}
+
+impl Drop for Foo {
+    fn drop(&mut self) {
+        if self.cnt == 1 {
+            FOO.with(|foo| assert_eq!(foo.cnt, 0));
+        } else {
+            assert_eq!(self.cnt, 0);
+            match FOO.state() {
+                LocalKeyState::Valid => panic!("should not be in valid state"),
+                LocalKeyState::Uninitialized |
+                LocalKeyState::Destroyed => {}
+            }
+        }
+    }
+}
+
+fn main() {
+    thread::spawn(|| {
+        FOO.with(|_| {});
+    }).join().unwrap();
+}