Auto merge of #25952 - alexcrichton:fix-scoped-tls, r=aturon

Currently the compiler has no knowledge of `#[thread_local]` which forces users
to take on two burdens of unsafety:

* The lifetime of the borrow of a `#[thread_local]` static is **not** `'static`
* Types in `static`s are required to be `Sync`

The thread-local modules mostly curb these facets of unsafety by only allowing
very limited scopes of borrows as well as allowing all types to be stored in a
thread-local key (regardless of whether they are `Sync`) through an `unsafe
impl`.

Unfortunately these measures have the consequence of being able to take the
address of the key itself and send it to another thread, allowing the same key
to be accessed from two different threads. This is clearly unsafe, and this
commit fixes this problem with the same trick used by `LocalKey`, which is to
have an indirect function call to find the address of the *current thread's*
thread local. This way the address of thread local keys can safely be sent among
threads as their lifetime truly is `'static`.

This commit will reduce the performance of cross-crate scoped thread locals as
it now requires an indirect function call, but this can likely be overcome in a
future commit.

Closes #25894

src/libstd/thread/mod.rs

@@ -217,6 +217,7 @@ pub use self::local::{LocalKey, LocalKeyState};
 pub use self::scoped_tls::ScopedKey;
 
 #[doc(hidden)] pub use self::local::__KeyInner as __LocalKeyInner;
+#[doc(hidden)] pub use self::scoped_tls::__KeyInner as __ScopedKeyInner;
 
 ////////////////////////////////////////////////////////////////////////////////
 // Builder

src/libstd/thread/scoped_tls.rs

@@ -43,6 +43,9 @@
 
 use prelude::v1::*;
 
+#[doc(hidden)]
+pub use self::imp::KeyInner as __KeyInner;
+
 /// Type representing a thread local storage key corresponding to a reference
 /// to the type parameter `T`.
 ///
@@ -53,7 +56,7 @@ use prelude::v1::*;
 #[unstable(feature = "scoped_tls",
            reason = "scoped TLS has yet to have wide enough use to fully consider \
                      stabilizing its interface")]
-pub struct ScopedKey<T> { inner: imp::KeyInner<T> }
+pub struct ScopedKey<T> { inner: fn() -> &'static imp::KeyInner<T> }
 
 /// Declare a new scoped thread local storage key.
 ///
@@ -64,42 +67,51 @@ pub struct ScopedKey<T> { inner: imp::KeyInner<T> }
 /// information.
 #[macro_export]
 #[allow_internal_unstable]
-#[cfg(not(no_elf_tls))]
 macro_rules! scoped_thread_local {
     (static $name:ident: $t:ty) => (
-        #[cfg_attr(not(any(windows,
-                           target_os = "android",
-                           target_os = "ios",
-                           target_os = "openbsd",
-                           target_arch = "aarch64")),
-                   thread_local)]
         static $name: ::std::thread::ScopedKey<$t> =
-            ::std::thread::ScopedKey::new();
+            __scoped_thread_local_inner!($t);
     );
     (pub static $name:ident: $t:ty) => (
-        #[cfg_attr(not(any(windows,
-                           target_os = "android",
-                           target_os = "ios",
-                           target_os = "openbsd",
-                           target_arch = "aarch64")),
-                   thread_local)]
         pub static $name: ::std::thread::ScopedKey<$t> =
-            ::std::thread::ScopedKey::new();
+            __scoped_thread_local_inner!($t);
     );
 }
 
+#[doc(hidden)]
+#[unstable(feature = "thread_local_internals",
+           reason = "should not be necessary")]
 #[macro_export]
 #[allow_internal_unstable]
 #[cfg(no_elf_tls)]
-macro_rules! scoped_thread_local {
+macro_rules! __scoped_thread_local_inner {
-    (static $name:ident: $t:ty) => (
+    ($t:ty) => {{
-        static $name: ::std::thread::ScopedKey<$t> =
+        static _KEY: ::std::thread::__ScopedKeyInner<$t> =
-            ::std::thread::ScopedKey::new();
+            ::std::thread::__ScopedKeyInner::new();
-    );
+        fn _getit() -> &'static ::std::thread::__ScopedKeyInner<$t> { &_KEY }
-    (pub static $name:ident: $t:ty) => (
+        ::std::thread::ScopedKey::new(_getit)
-        pub static $name: ::std::thread::ScopedKey<$t> =
+    }}
-            ::std::thread::ScopedKey::new();
+}
-    );
+
+#[doc(hidden)]
+#[unstable(feature = "thread_local_internals",
+           reason = "should not be necessary")]
+#[macro_export]
+#[allow_internal_unstable]
+#[cfg(not(no_elf_tls))]
+macro_rules! __scoped_thread_local_inner {
+    ($t:ty) => {{
+        #[cfg_attr(not(any(windows,
+                           target_os = "android",
+                           target_os = "ios",
+                           target_os = "openbsd",
+                           target_arch = "aarch64")),
+                   thread_local)]
+        static _KEY: ::std::thread::__ScopedKeyInner<$t> =
+            ::std::thread::__ScopedKeyInner::new();
+        fn _getit() -> &'static ::std::thread::__ScopedKeyInner<$t> { &_KEY }
+        ::std::thread::ScopedKey::new(_getit)
+    }}
 }
 
 #[unstable(feature = "scoped_tls",
@@ -107,8 +119,8 @@ macro_rules! scoped_thread_local {
                      stabilizing its interface")]
 impl<T> ScopedKey<T> {
     #[doc(hidden)]
-    pub const fn new() -> ScopedKey<T> {
+    pub const fn new(inner: fn() -> &'static imp::KeyInner<T>) -> ScopedKey<T> {
-        ScopedKey { inner: imp::KeyInner::new() }
+        ScopedKey { inner: inner }
     }
 
     /// Inserts a value into this scoped thread local storage slot for a
@@ -153,13 +165,14 @@ impl<T> ScopedKey<T> {
             }
         }
 
+        let inner = (self.inner)();
         let prev = unsafe {
-            let prev = self.inner.get();
+            let prev = inner.get();
-            self.inner.set(t as *const T as *mut T);
+            inner.set(t as *const T as *mut T);
             prev
         };
 
-        let _reset = Reset { key: &self.inner, val: prev };
+        let _reset = Reset { key: inner, val: prev };
         cb()
     }
 
@@ -186,7 +199,7 @@ impl<T> ScopedKey<T> {
         F: FnOnce(&T) -> R
     {
         unsafe {
-            let ptr = self.inner.get();
+            let ptr = (self.inner)().get();
             assert!(!ptr.is_null(), "cannot access a scoped thread local \
                                      variable without calling `set` first");
             cb(&*ptr)
@@ -195,7 +208,7 @@ impl<T> ScopedKey<T> {
 
     /// Test whether this TLS key has been `set` for the current thread.
     pub fn is_set(&'static self) -> bool {
-        unsafe { !self.inner.get().is_null() }
+        unsafe { !(self.inner)().get().is_null() }
     }
 }
 
@@ -205,6 +218,7 @@ impl<T> ScopedKey<T> {
               target_os = "openbsd",
               target_arch = "aarch64",
               no_elf_tls)))]
+#[doc(hidden)]
 mod imp {
     use std::cell::Cell;
 
@@ -227,6 +241,7 @@ mod imp {
           target_os = "openbsd",
           target_arch = "aarch64",
           no_elf_tls))]
+#[doc(hidden)]
 mod imp {
     use prelude::v1::*;