Can't use parking_lot primitives in thread destructors (TLS variable's Drop)

[ticki]

It seems that parking_lot makes use of TLS vars, which subtly makes the primitives unusable in some cases of thread destructors, since the internal TLS variable has been dropped. Ideally, a secondary path should exist, taken if the TLS variable has been destroyed.

[Amanieu]

TLS is mainly used for the THREAD_DATA structures, which only need to exist while a thread is waiting. The reason these are stored as TLS instead of just using a local variable is because allocating a new ThreadParker may be an expensive operation (Mutex::new/Condvar::new, which both allocate memory). However if the TLS slot is marked as destroyed, we can just use a local variable instead. This is possible on nightly only by using LocalKey::try_with.

[Amanieu]

Fixed in 0.4.5

[ticki]

Doesn't appear to be fixed:

thread '<unnamed>' panicked at 'cannot access a TLS value during or after it is destroyed: AccessError', /checkout/src/libco
re/result.rs:860:4
stack backtrace:
   0: std::sys::imp::backtrace::tracing::imp::unwind_backtrace
             at /checkout/src/libstd/sys/unix/backtrace/tracing/gcc_s.rs:49
   1: std::sys_common::backtrace::_print
             at /checkout/src/libstd/sys_common/backtrace.rs:71
   2: std::panicking::default_hook::{{closure}}
             at /checkout/src/libstd/sys_common/backtrace.rs:60
             at /checkout/src/libstd/panicking.rs:380
   3: std::panicking::default_hook
             at /checkout/src/libstd/panicking.rs:396
   4: std::panicking::rust_panic_with_hook
             at /checkout/src/libstd/panicking.rs:611
   5: std::panicking::begin_panic_new
             at /checkout/src/libstd/panicking.rs:553
   6: std::panicking::begin_panic_fmt
             at /checkout/src/libstd/panicking.rs:521
   7: rust_begin_unwind
             at /checkout/src/libstd/panicking.rs:497
   8: core::panicking::panic_fmt
             at /checkout/src/libcore/panicking.rs:92
   9: core::result::unwrap_failed
             at /checkout/src/libcore/macros.rs:31
  10: <core::result::Result<T, E>>::expect
             at /checkout/src/libcore/result.rs:762
  11: <std::thread::local::LocalKey<T>>::with
             at /checkout/src/libstd/thread/local.rs:343
  12: parking_lot_core::parking_lot::park_internal
             at /home/ticki/.local/share/cargo/registry/src/github.com-1ecc6299db9ec823/parking_lot_core0.2.2/src/parking_lot.rs:515
  13: parking_lot_core::parking_lot::park
             at /home/ticki/.local/share/cargo/registry/src/github.com-1ecc6299db9ec823/parking_lot_core0.2.2/src/parking_lot.rs:498
  14: parking_lot::raw_mutex::RawMutex::lock_slow
             at /home/ticki/.local/share/cargo/registry/src/github.com-1ecc6299db9ec823/parking_lot0.4.5/src/raw_mutex.rs:176
  15: parking_lot::raw_mutex::RawMutex::lock
             at /home/ticki/.local/share/cargo/registry/src/github.com-1ecc6299db9ec823/parking_lot0.4.5/src/raw_mutex.rs:53
  16: <parking_lot::mutex::Mutex<T>>::lock
             at /home/ticki/.local/share/cargo/registry/src/github.com-1ecc6299db9ec823/parking_lot0.4.5/src/mutex.rs:156
  17: <conc::mpsc::Sender<T>>::send
             at src/mpsc.rs:39
  18: conc::global::State::export_garbage
             at src/global.rs:113
  19: conc::global::export_garbage
             at src/global.rs:29
  20: conc::local::State::export_garbage
             at src/local.rs:193
  21: <conc::local::State as core::ops::drop::Drop>::drop
             at src/local.rs:209
  22: core::ptr::drop_in_place
             at /checkout/src/libcore/ptr.rs:60
  23: core::ptr::drop_in_place
             at /checkout/src/libcore/ptr.rs:60
  24: core::ptr::drop_in_place
             at /checkout/src/libcore/ptr.rs:60
  25: core::ptr::drop_in_place
             at /checkout/src/libcore/ptr.rs:60
  26: std::thread::local::fast::destroy_value
             at /checkout/src/libstd/thread/local.rs:508
  27: __GI___call_tls_dtors
  28: start_thread
  29: clone

[ticki]

Reproduction (through redox-os/tfs):

diff --git a/conc/Cargo.toml b/conc/Cargo.toml
index f541f3c..65bc2f2 100644
--- a/conc/Cargo.toml
+++ b/conc/Cargo.toml
@@ -12,7 +12,7 @@ exclude = ["target", "Cargo.lock"]
 [dependencies]
 lazy_static = "0.2"
 rand = "0.3"
-spin = "0.4"
+parking_lot = "0.4"

 [dependencies.backtrace]
 version = "0.3"
diff --git a/conc/src/global.rs b/conc/src/global.rs
index 86b07d7..21d7da0 100644
--- a/conc/src/global.rs
+++ b/conc/src/global.rs
@@ -1,6 +1,6 @@
 //! The global state.

-use spin::Mutex;
+use parking_lot::Mutex;
 use std::collections::HashSet;
 use std::{mem, panic};
 use {rand, hazard, mpsc, debug, settings};
diff --git a/conc/src/lib.rs b/conc/src/lib.rs
index 2faf7c9..18e4d6a 100644
--- a/conc/src/lib.rs
+++ b/conc/src/lib.rs
@@ -150,7 +150,7 @@
 #[macro_use]
 extern crate lazy_static;
 extern crate rand;
-extern crate spin;
+extern crate parking_lot;

 mod atomic;
 mod debug;
diff --git a/conc/src/mpsc.rs b/conc/src/mpsc.rs
index 1596836..7820864 100644
--- a/conc/src/mpsc.rs
+++ b/conc/src/mpsc.rs
@@ -8,7 +8,7 @@
 //! although this is reasonably fast as the lock is only held for very short time, it is
 //! sub-optimal, and blocking.

-use spin::Mutex;
+use parking_lot::Mutex;
 use std::sync::Arc;
 use std::mem;

[ticki]

(just run test suite of conc)

[Amanieu]

Ah sorry, forgot to cargo publish parking_lot_core. Should work now with parking_lot_core v0.2.3.

[ticki]

I still get a bunch of errors, albeit the tests runs:

thread '<unnamed>' panicked at 'cannot access a TLS value during or after it is destroyed: AccessError', /checkout/src/libcore/result.rs:860:4                                                 
thread '<unnamed>' panicked at 'cannot access a TLS value during or after it is destroyed: AccessError', /checkout/src/libcore/result.rs:860:4                                                 
thread '<unnamed>' panicked at 'cannot access a TLS value during or after it is destroyed: AccessError', /checkout/src/libcore/result.rs:860:4                                                 
thread '<unnamed>' panicked at 'cannot access a TLS value during or after it is destroyed: AccessError', /checkout/src/libcore/result.rs:860:4                                                 
thread '<unnamed>' panicked at 'cannot access a TLS value during or after it is destroyed: AccessError', /checkout/src/libcore/result.rs:860:4                                                 
thread '<unnamed>' panicked at 'cannot access a TLS value during or after it is destroyed: AccessError', /checkout/src/libcore/result.rs:860:4                                                 
thread '<unnamed>' panicked at 'cannot access a TLS value during or after it is destroyed: AccessError', /checkout/src/libcore/result.rs:860:4                                                 
thread '<unnamed>' panicked at 'cannot access a TLS value during or after it is destroyed: AccessError', /checkout/src/libcore/result.rs:860:4     

[Amanieu]

I use catch_panic to catch the panic when accessing a TLS variable. If you enable the nightly feature then it will use LocalKey::try_with, which doesn't trigger a panic in the first place.

[ticki]

Ah I see.