From fdd1b58452d5f259b3f18a5a0e0b1f4624cfdcbe Mon Sep 17 00:00:00 2001
From: AzureMarker <mark.drobnak@gmail.com>
Date: Sat, 5 Feb 2022 22:28:44 -0800
Subject: [PATCH] Fix reentrant locking behavior in thread keys

The standard library RwLock checks if it's panicking while making the
guard. This check needs to use a thread local key, which it lazily
creates. This causes reentrant/recursive locking, which leads to a
deadlock.

This commit replaces the std RwLock with one from spin, which does not
need to check thread local storage to lock. It's a spin-lock, but it is
configured to yield the thread on every unsuccessful spin. Also, the
lock can be created statically, which means we don't need once_cell.

An alternative to spin is parking_lot, but that has some compile errors
(it wants condvars to support monotonic clocks) and brings in more
dependencies.
---
 Cargo.toml |  2 +-
 src/lib.rs | 20 ++++++--------------
 2 files changed, 7 insertions(+), 15 deletions(-)

diff --git a/Cargo.toml b/Cargo.toml
index 18434e0..54b894a 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -7,4 +7,4 @@ edition = "2021"
 
 [dependencies]
 libc = "0.2.116"
-once_cell = "1.0"
\ No newline at end of file
+spin = { version = "0.9", default-features = false, features = ["rwlock", "std"] }
\ No newline at end of file
diff --git a/src/lib.rs b/src/lib.rs
index 137a85f..ad6a955 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -399,26 +399,24 @@ pub unsafe extern "C" fn pthread_rwlockattr_destroy(
 
 // THREAD KEYS IMPLEMENTATION FOR RUST STD
 
-use once_cell::sync::Lazy;
+use spin::rwlock::RwLock;
 use std::collections::BTreeMap;
 use std::ptr;
 use std::sync::atomic::{AtomicUsize, Ordering};
-use std::sync::{PoisonError, RwLock};
 
 type Key = usize;
 type Destructor = unsafe extern "C" fn(*mut libc::c_void);
 
 static NEXT_KEY: AtomicUsize = AtomicUsize::new(1);
 
-static KEYS: Lazy<RwLock<BTreeMap<Key, Option<Destructor>>>> = Lazy::new(RwLock::default);
+// This is a spin-lock RwLock which yields the thread every loop
+static KEYS: RwLock<BTreeMap<Key, Option<Destructor>>, spin::Yield> = RwLock::new(BTreeMap::new());
 
 #[thread_local]
 static mut LOCALS: BTreeMap<Key, *mut libc::c_void> = BTreeMap::new();
 
 fn is_valid_key(key: Key) -> bool {
-    KEYS.read()
-        .unwrap_or_else(PoisonError::into_inner)
-        .contains_key(&(key as Key))
+    KEYS.read().contains_key(&(key as Key))
 }
 
 #[no_mangle]
@@ -427,9 +425,7 @@ pub unsafe extern "C" fn pthread_key_create(
     destructor: Option<Destructor>,
 ) -> libc::c_int {
     let new_key = NEXT_KEY.fetch_add(1, Ordering::SeqCst);
-    KEYS.write()
-        .unwrap_or_else(PoisonError::into_inner)
-        .insert(new_key, destructor);
+    KEYS.write().insert(new_key, destructor);
 
     *key = new_key as libc::pthread_key_t;
 
@@ -438,11 +434,7 @@ pub unsafe extern "C" fn pthread_key_create(
 
 #[no_mangle]
 pub unsafe extern "C" fn pthread_key_delete(key: libc::pthread_key_t) -> libc::c_int {
-    match KEYS
-        .write()
-        .unwrap_or_else(PoisonError::into_inner)
-        .remove(&(key as Key))
-    {
+    match KEYS.write().remove(&(key as Key)) {
         // We had a entry, so it was a valid key.
         // It's officially undefined behavior if they use the key after this,
         // so don't worry about cleaning up LOCALS, especially since we can't