Address review comments

Author: orlp

library/std/src/sys/thread_local/destructors/list.rs

@@ -7,7 +7,9 @@ static DTORS: RefCell<Vec<(*mut u8, unsafe extern "C" fn(*mut u8)), System>> =
     RefCell::new(Vec::new_in(System));
 
 pub unsafe fn register(t: *mut u8, dtor: unsafe extern "C" fn(*mut u8)) {
-    let Ok(mut dtors) = DTORS.try_borrow_mut() else { rtabort!("unreachable") };
+    let Ok(mut dtors) = DTORS.try_borrow_mut() else {
+        rtabort!("the System allocator may not use TLS with destructors")
+    };
     guard::enable();
     dtors.push((t, dtor));
 }

library/std/src/thread/current.rs

@@ -249,23 +249,13 @@ fn init_current(current: *mut ()) -> Thread {
         // BUSY exists solely for this check, but as it is in the slow path, the
         // extra TLS write above shouldn't matter. The alternative is nearly always
         // a stack overflow.
-
-        // If you came across this message, contact the author of your
-        // allocator. If you are said author: A surprising amount of functions
-        // inside the standard library (e.g. `Mutex`, `File` when using long
-        // paths, even `panic!` when using unwinding), need memory allocation,
-        // so you'll get circular dependencies all over the place when using
-        // them. I (joboet) highly recommend using only APIs from core in your
-        // allocator and implementing your own system abstractions. Still, if
-        // you feel that a particular API should be entirely allocation-free,
-        // feel free to open an issue on the Rust repository, we'll see what we
-        // can do.
+        //
+        // If we reach this point it means our initialization routine ended up
+        // calling current() either directly, or indirectly through the global
+        // allocator, which is a bug either way as we may not call the global
+        // allocator in current().
         rtabort!(
-            "\n\
-            Attempted to access thread-local data while allocating said data.\n\
-            Do not access functions that allocate in the global allocator!\n\
-            This is a bug in the global allocator.\n\
-            "
+            "init_current() was re-entrant, which indicates a bug in the Rust threading implementation"
         )
     } else {
         debug_assert_eq!(current, DESTROYED);

library/std/src/thread/mod.rs

@@ -1303,20 +1303,17 @@ impl ThreadId {
                     spin += 1;
                 }
 
-                let id;
                 // SAFETY: we have an exclusive lock on the counter.
                 unsafe {
-                    id = (*COUNTER.get()).saturating_add(1);
-                    (*COUNTER.get()) = id;
-                };
-
-                // Release the lock.
-                COUNTER_LOCKED.store(false, Ordering::Release);
-
-                if id == u64::MAX {
-                    exhausted()
-                } else {
-                    ThreadId(NonZero::new(id).unwrap())
+                    if *COUNTER.get() == u64::MAX {
+                        COUNTER_LOCKED.store(false, Ordering::Release);
+                        exhausted()
+                    } else {
+                        let id = *COUNTER.get() + 1;
+                        *COUNTER.get() = id;
+                        COUNTER_LOCKED.store(false, Ordering::Release);
+                        ThreadId(NonZero::new(id).unwrap())
+                    }
                 }
             }
         }