Use ThreadId instead of TLS-address in `ReentrantLock` #124881

Sp00ph · 2024-05-08T08:48:35Z

ReentrantLock currently uses the address of a thread local variable as an ID that's unique across all currently running threads. This can lead to uninituitive behavior as in #123458 if TLS blocks get reused. This PR changes ReentrantLock to instead use the ThreadId provided by std as the unique ID. ThreadId guarantees uniqueness across the lifetime of the whole process, so we don't need to worry about reusing IDs of terminated threads. The main appeal of this PR is thus the possibility of changing the ReentrantLock API to guarantee that if a thread leaks a lock guard, no other thread may ever acquire that lock again.

This does entail some complications:

previously, the only way to retrieve the current thread ID would've been using thread::current().id() which creates a temporary Arc and which isn't available in TLS destructors. As part of this PR, the thread ID instead gets cached in its own thread local, as suggested here.
ThreadId is always 64-bit whereas the current implementation uses a usize-sized ID. Since this ID needs to be updated atomically, we can't simply use a single atomic variable on 32 bit platforms. Instead, we fall back to using a (sound) seqlock on 32-bit platforms, which works because only one thread at a time can write to the ID. This seqlock is technically susceptible to the ABA problem, but the attack vector to create actual unsoundness has to be very specific:
- You would need to be able to lock+unlock the lock exactly 2^31 times (or a multiple thereof) while a thread trying to lock it sleeps
- The sleeping thread would have to suspend after reading one half of the thread id but before reading the other half
- The teared result from combining the halves of the thread ID would have to exactly line up with the sleeping thread's ID

The risk of this occurring seems slim enough to be acceptable to me, but correct me if I'm wrong. This also means that the size of the lock increases by 8 bytes on 32-bit platforms, but this also shouldn't be an issue.

Performance wise, I did some crude testing of the only case where this could lead to real slowdowns, which is the case of locking a ReentrantLock that's already locked by the current thread. On both aarch64 and x86-64, there is (expectedly) pretty much no performance hit. I didn't have any 32-bit platforms to test the seqlock performance on, so I did the next best thing and just forced the 64-bit platforms to use the seqlock implementation. There, the performance degraded by ~1-2ns/(lock+unlock) on x86-64 and ~6-8ns/(lock+unlock) on aarch64, which is measurable but seems acceptable to me seeing as 32-bit platforms should be a small minority anyways.

cc @joboet @RalfJung @CAD97

RalfJung · 2024-05-08T08:58:56Z

32bit x86 has max_atomic_width set to 64. So even on those targets we won't be using the seqlock.

RalfJung · 2024-05-08T09:02:48Z

library/std/src/sync/reentrant_lock.rs

+        panic!("Thread hasn't been assigned an ID!")
+    }
+
+    crate::thread::try_current_id().map_or_else(|| no_tid(), |tid| tid.as_u64().get())


I think this is more readable as

Suggested change

crate::thread::try_current_id().map_or_else(|| no_tid(), |tid| tid.as_u64().get())

crate::thread::try_current_id().unwrap_or_else(|| no_tid()).as_u64().get()

RalfJung · 2024-05-08T09:04:26Z

library/std/src/thread/mod.rs

+    CURRENT
+        .try_with(|current| {
+            let thread = current.get_or_init(Thread::new_unnamed).clone();
+            CURRENT_ID.set(Some(thread.id()));


So this does a set over and over again? Isn't that excessive?

Oh right, not sure how I didn't catch that. I suppose I can just move the CURRENT_ID.set() into the get_or_init() so it's only executed once.

RalfJung · 2024-05-08T09:05:22Z

library/std/src/thread/mod.rs

@@ -699,16 +699,18 @@ where

 thread_local! {
    static CURRENT: OnceCell<Thread> = const { OnceCell::new() };
+    static CURRENT_ID: Cell<Option<ThreadId>> = const { Cell::new(None) };


So now we redundantly store the thread ID twice, once in the Thread and once here?

At least we should explicitly document this invariant. Ideally we can avoid the redundancy...

I'm not sure the redundancy can be avoided, seeing as we want to access the thread local without accessing the Thread and vice versa. And since the Thread instance can outlive the thread itself, we also can't store a reference to the thread local in the Thread instance. Maybe we could store something like a UnsendUnsync<&'static Thread> in a thread local pointing to the current Thread instance? That way we could at least avoid the Arc clone + drop that occurs with thread::current().id()...

RalfJung · 2024-05-08T09:06:33Z

library/std/src/thread/mod.rs

+#[inline]
+pub(crate) fn try_current_id() -> Option<ThreadId> {
+    if CURRENT_ID.get().is_none() {
+        let _ = try_current();


This will access CURRENT, is that really what we want? I thought the goal is to not touch CURRENT at all in this function.

It'll only access CURRENT at most once, when the CURRENT_ID is unset. In that case, accessing CURRENT is unavoidable, because one way or another you need to access the thread id. try_current then initializes the CURRENT_ID so that subsequent try_current_id() calls don't need to call try_current() again.

Right, the entire lifecycle here is kind of subtle... and there are no comments to help the reader.

There's also the invariant that CURRENT and CURRENT_ID are always initialized together, I think?

Yeah, they're always initialized together (but CURRENT_ID stays initialized even for TLS dtors, unlike CURRENT). I mostly just copied @CAD97's code from #123458, but I can definitely add some comments later.

RalfJung · 2024-05-08T09:37:01Z

library/std/src/sync/reentrant_lock.rs

+pub(crate) fn current_thread_id() -> u64 {
+    #[cold]
+    fn no_tid() -> ! {
+        panic!("Thread hasn't been assigned an ID!")


This should probably be rtabort! or so, panic! may call user code which may try to print things which may try to do reentrant lock stuff again.

Hm, is there a meaningful difference between this and something like RwLock::read (which panics when the reader count hits the max value)? In both cases, if you try to relock it then it'll just panic again.

IIRC ReentrantLock is used deep in the guts of std, unlike RwLock? Also, hitting the max value is much less likely than being in a thread that was spawned by non-Rust code. (Other parts of your PR even assume for correctness that such overflow never happens.)

IIRC ReentrantLock is used deep in the guts of std, unlike RwLock?

That makes sense, I'll change it to an rtabort! then.

Other parts of your PR even assume for correctness that such overflow never happens.

Are you referring to the seq counter? That's the only place I can think of where overflow can matter and that can only cause problems in the exact case described in the PR. There's also precedence for this, e.g. Condvar also operates like this (though you could argue that the fallout from hitting this case in Condvar is smaller as it just keeps a thread asleep indefinitely instead of potentially causing unsoundness).

joboet

Frankly I'm still not entirely convinced that this should be fixed at all, I just don't see what you would want this guarantee for. I struggle to come up with any usecase that couldn't be better solved another way.

@rustbot label +I-libs-api-nominated

As for the implementation, I think you could still use word-sized IDs by recording the number of currently locked ReentrantLocks in a TLS variable and leaking the current ID when that number is non-zero on thread exit. But you don't have to do that.

joboet · 2024-05-08T13:10:56Z

library/std/src/sync/reentrant_lock.rs

+                        crate::hint::spin_loop();
+                        seq = self.seq.load(Acquire);


No need to spin here: if there is a write in progress, the current thread cannot be the lock owner.

joboet · 2024-05-08T13:18:30Z

library/std/src/sync/reentrant_lock.rs

+        }
+
+        impl Tid {
+            const fn new(tid: u64) -> Self {


This is subjective, but for me this code would be more readable if the ThreadId abstraction was only taken away inside here. Upon first glance, this PR seemed unsound to me because I didn't remember that ThreadIds are guaranteed to be non-zero, using u64 instead of Option<ThreadId> in the comparison makes the logic clearer. May I suggest doing

Suggested change

const fn new(tid: u64) -> Self {

const fn new(tid: Option<ThreadId>) -> Self {

and so on?

Sp00ph · 2024-05-25T01:59:44Z

@rustbot ready

Sp00ph · 2024-07-16T20:31:58Z

bump

joboet

Mara is on vacation, so
r? @joboet

I changed my mind, this makes sense. The implementation looks good, but I think it can be slightly improved, if you have the time. Otherwise, I'll just approve this.

joboet · 2024-07-17T13:28:17Z

library/std/src/sync/reentrant_lock.rs

+            seq: AtomicU32,
+            low: AtomicU32,
+            high: AtomicU32,


It just occurred to me: Instead of a seq-lock you could use the thread-local address as a fast check and only if it matches check the thread ID. Since the thread-local address is unique for a thread at any given time, there is no need to synchronize the thread ID, so it can be inside a Cell.

joboet · 2024-07-17T13:33:52Z

library/std/src/thread/mod.rs

+    if CURRENT_ID.get().is_none() {
+        // If `CURRENT_ID` isn't initialized yet, then `CURRENT` must also not be initialized.
+        // `try_current()` will try to initialize both `CURRENT` and `CURRENT_ID`.
+        // Subsequent calls to `try_current_id` will then no longer enter this if-branch.
+        let _ = try_current();
+    }
+    CURRENT_ID.get()


Suggested change

if CURRENT_ID.get().is_none() {

// If `CURRENT_ID` isn't initialized yet, then `CURRENT` must also not be initialized.

// `try_current()` will try to initialize both `CURRENT` and `CURRENT_ID`.

// Subsequent calls to `try_current_id` will then no longer enter this if-branch.

let _ = try_current();

}

CURRENT_ID.get()

CURRENT_ID.get().unwrap_or_else(|| {

// If `CURRENT_ID` isn't initialized yet, then `CURRENT` must also not be initialized. `current()` will initialize both `CURRENT` and `CURRENT_ID` so subsequent calls to `id` will succeed immediately.

current().id()

})

joboet · 2024-07-17T13:35:31Z

library/std/src/thread/mod.rs

@@ -698,17 +698,22 @@ where
 }

 thread_local! {
+    // Invariant: `CURRENT` and `CURRENT_ID` will always be initialized
+    // together. However, while `CURRENT_ID` will be available during
+    // TLS constructors, `CURRENT` will not.


I think you mean destructors? And this isn't quite true, key-based TLS is always destroyed (but can be reinitialized, so this isn't that much of a problem).

joboet · 2024-07-17T13:48:14Z

library/std/src/thread/mod.rs

+/// If called from inside a TLS destructor and the thread was never
+/// assigned an id, returns `None`.
+#[inline]
+pub(crate) fn try_current_id() -> Option<ThreadId> {


That's not true. If CURRENT hasn't been initialized yet, it will be initialized, even in a TLS destructor. If it has been initialized, and we are inside its destructor, then CURRENT_ID has been set already. So the only place where this can fail is if the global allocator calls this function while trying to deallocate the TLS storage for the current ID. The global allocator shouldn't be doing that anyway, so we can just panic in that case. Therefore, this function can be infallible (I slightly prefer the name id, like with current the module and signature alone suffice IMHO):

Suggested change

/// If called from inside a TLS destructor and the thread was never

/// assigned an id, returns `None`.

#[inline]

pub(crate) fn try_current_id() -> Option<ThreadId> {

#[inline]

pub(crate) fn id() -> ThreadId {

Sp00ph · 2024-07-18T00:10:06Z

I believe this addresses all your comments.

joboet

Looks good, just some small nits.

joboet · 2024-07-18T10:47:44Z

library/std/src/sync/reentrant_lock.rs

+// On systems without 64 bit atomics we use a simple seqlock to emulate a 64 bit Tid using
+// 32 bit atomics (which should be supported on all platforms with `std`). This works
+// because only one thread at a time (the one holding the mutex) writes to it.


This comment needs an update.

joboet · 2024-07-18T10:49:02Z

library/std/src/sync/reentrant_lock.rs

+
+            #[inline]
+            // This may only be called by one thread at a time.
+            fn set(&self, tid: Option<ThreadId>) {


That's a safety precondition, so I think this needs to be unsafe?

joboet · 2024-07-18T10:50:18Z

library/std/src/thread/mod.rs

@@ -698,17 +698,20 @@ where
 }

 thread_local! {
+    // Invariant: `CURRENT` and `CURRENT_ID` will always be initialized together.


Could you also add a note about CURRENT_ID always holding the same ID as the thread in CURRENT, if it is initialized?

joboet · 2024-07-18T13:51:47Z

Great, thank you! Please squash the commits and then r=me.

@bors delegate+

bors · 2024-07-18T13:51:50Z

✌️ @Sp00ph, you can now approve this pull request!

If @joboet told you to "r=me" after making some further change, please make that change, then do @bors r=@joboet

This changes `ReentrantLock` to use `ThreadId` for the thread ownership check instead of the address of a thread local. Unlike TLS blocks, `ThreadId` is guaranteed to be unique across the lifetime of the process, so if any thread ever terminates while holding a `ReentrantLockGuard`, no other thread may ever acquire that lock again. On platforms with 64-bit atomics, this is a very simple change. On other platforms, the approach used is slightly more involved, as explained in the module comment. This also adds a `CURRENT_ID` thread local in addition to the already existing `CURRENT`. This allows us to access the current `ThreadId` without the relatively heavy machinery used by `thread::current().id()`.

Sp00ph · 2024-07-18T14:14:29Z

@bors r=joboet

bors · 2024-07-18T14:14:32Z

📌 Commit fe89962 has been approved by joboet

It is now in the queue for this repository.

…iaskrgr Rollup of 6 pull requests Successful merges: - rust-lang#124881 (Use ThreadId instead of TLS-address in `ReentrantLock`) - rust-lang#127656 (make pub_use_of_private_extern_crate show up in cargo's future breakage reports) - rust-lang#127748 (Use Option's discriminant as its size hint) - rust-lang#127854 (Add internal lint for detecting non-glob imports of `rustc_type_ir::inherent`) - rust-lang#127908 (Update extern linking documentation) - rust-lang#127919 (Allow a git command for getting the current branch in bootstrap to fail) r? `@ghost` `@rustbot` modify labels: rollup

Rollup merge of rust-lang#124881 - Sp00ph:reentrant_lock_tid, r=joboet Use ThreadId instead of TLS-address in `ReentrantLock` Fixes rust-lang#123458 `ReentrantLock` currently uses the address of a thread local variable as an ID that's unique across all currently running threads. This can lead to uninituitive behavior as in rust-lang#123458 if TLS blocks get reused. This PR changes `ReentrantLock` to instead use the `ThreadId` provided by `std` as the unique ID. `ThreadId` guarantees uniqueness across the lifetime of the whole process, so we don't need to worry about reusing IDs of terminated threads. The main appeal of this PR is thus the possibility of changing the `ReentrantLock` API to guarantee that if a thread leaks a lock guard, no other thread may ever acquire that lock again. This does entail some complications: - previously, the only way to retrieve the current thread ID would've been using `thread::current().id()` which creates a temporary `Arc` and which isn't available in TLS destructors. As part of this PR, the thread ID instead gets cached in its own thread local, as suggested [here](rust-lang#123458 (comment)). - `ThreadId` is always 64-bit whereas the current implementation uses a usize-sized ID. Since this ID needs to be updated atomically, we can't simply use a single atomic variable on 32 bit platforms. Instead, we fall back to using a (sound) seqlock on 32-bit platforms, which works because only one thread at a time can write to the ID. This seqlock is technically susceptible to the ABA problem, but the attack vector to create actual unsoundness has to be very specific: - You would need to be able to lock+unlock the lock exactly 2^31 times (or a multiple thereof) while a thread trying to lock it sleeps - The sleeping thread would have to suspend after reading one half of the thread id but before reading the other half - The teared result from combining the halves of the thread ID would have to exactly line up with the sleeping thread's ID The risk of this occurring seems slim enough to be acceptable to me, but correct me if I'm wrong. This also means that the size of the lock increases by 8 bytes on 32-bit platforms, but this also shouldn't be an issue. Performance wise, I did some crude testing of the only case where this could lead to real slowdowns, which is the case of locking a `ReentrantLock` that's already locked by the current thread. On both aarch64 and x86-64, there is (expectedly) pretty much no performance hit. I didn't have any 32-bit platforms to test the seqlock performance on, so I did the next best thing and just forced the 64-bit platforms to use the seqlock implementation. There, the performance degraded by ~1-2ns/(lock+unlock) on x86-64 and ~6-8ns/(lock+unlock) on aarch64, which is measurable but seems acceptable to me seeing as 32-bit platforms should be a small minority anyways. cc `@joboet` `@RalfJung` `@CAD97`

9cc3bc6 custom MIR: add support for tail calls 5674d1c Auto merge of rust-lang#128673 - matthiaskrgr:rollup-gtvpkm7, r=matthiaskrgr deb1d75 Rollup merge of rust-lang#128619 - glandium:last_chunk, r=scottmcm 6449537 Rollup merge of rust-lang#128609 - swenson:smaller-faster-dragon, r=Amanieu acb2c30 Rollup merge of rust-lang#128026 - devnexen:available_parallelism_vxworks, r=Mark-Simulacrum 89fe6df Rollup merge of rust-lang#128309 - kmicklas:btreeset-cursor, r=Amanieu 313484b Correct the const stabilization of `<[T]>::last_chunk` 22e026b Auto merge of rust-lang#128534 - bjorn3:split_stdlib_workspace, r=Mark-Simulacrum 1813603 Rollup merge of rust-lang#128526 - tshepang:patch-1, r=Amanieu e8a1a41 Auto merge of rust-lang#128466 - sayantn:stdarch-update, r=tgross35 2adf9da Update stdarch dc85bdb Chore: add `x86_amx_intrinsics` feature flag to `core/lib.rs` and remove `issue-120720-reduce-nan.rs` e88b04d Rollup merge of rust-lang#128551 - Konippi:refactor-backtrace-style-in-panic, r=tgross35 43a1e93 Rollup merge of rust-lang#128530 - scottmcm:repeat-n-unchecked, r=joboet 47df194 Remove unnecessary constants from flt2dec dragon 0b5f1b8 Auto merge of rust-lang#128404 - compiler-errors:revert-dead-code-changes, r=pnkfelix 35cd95f Suppress new false-negatives that were masked by dead code analysis changes 9eb9fa6 Revert "Rollup merge of rust-lang#127107 - mu001999-contrib:dead/enhance-2, r=pnkfelix" 975dc19 Rollup merge of rust-lang#128368 - nnethercote:rustfmt-tweaks, r=cuviper 0ee09fe Rollup merge of rust-lang#128303 - NobodyXu:specialise-for-pipe, r=cuviper 1967a12 Rollup merge of rust-lang#127586 - zachs18:more-must-use, r=cuviper fc53324 Rollup merge of rust-lang#126704 - sayantn:sha, r=Amanieu 74dd96f chore: refactor backtrace style in panic 0e4358d Auto merge of rust-lang#128528 - workingjubilee:you-dont-need-to-see-this-cpuid-move-along, r=Amanieu 9fa74ab Move the standard library to a separate workspace e13d132 Auto merge of rust-lang#128254 - Amanieu:orig-binary-search, r=tgross35 05d8d7c Implement `UncheckedIterator` directly for `RepeatN` a5fa13e Rollup merge of rust-lang#128491 - c410-f3r:unlock-rfc-2011, r=workingjubilee 28e4d22 Rollup merge of rust-lang#128453 - RalfJung:raw_eq, r=saethlin db770c6 std: Remove has_cpuid 39aad04 time.rs: remove "Basic usage text" 7df583c Dogfood cb11051 Add the `sha512`, `sm3` and `sm4` target features 86ea79f Fix mutability in doc tests for `BTreeSet` cursors c7be27f Introduce `Cursor`/`CursorMut`/`CursorMutKey` thrichotomy for `BTreeSet` like map API 8835b0f Fix some uses of "map" instead of "set" in `BTreeSet` cursor API docs 07f64a8 Share `UnorderedKeyError` with `BTReeMap` for set API f859e54 Rollup merge of rust-lang#128499 - Konippi:refactor-backtrace-formatting, r=tgross35 fb966d2 Rollup merge of rust-lang#128497 - Bryanskiy:fix-dropck-doc, r=lcnr ab00ae6 Rollup merge of rust-lang#128433 - hermit-os:hermit-unsafe_op_in_unsafe_fn, r=joboet 2614bd2 chore: refactor backtrace formatting a8a4659 fix dropck documentation for `[T;0]` special-case 589c0a0 fix(os/hermit): `deny(unsafe_op_in_unsafe_fn)` 0260e47 fix(pal/hermit): `deny(unsafe_op_in_unsafe_fn)` 7bd6b11 refactor(pal/hermit): make `ENV` a non-mutable static 32894e2 Rollup merge of rust-lang#128416 - maurer:remove-android-hack, r=tgross35 beb76c3 Auto merge of rust-lang#128461 - matthiaskrgr:rollup-3dpp11g, r=matthiaskrgr 1e3976b Rollup merge of rust-lang#128162 - ChrisDenton:cleanup, r=joboet cde45b0 Rollup merge of rust-lang#127567 - joboet:once_wait, r=Amanieu 0607642 Fix docs for OnceLock::get_mut_or_init da48417 raw_eq: using it on bytes with provenance is not UB (outside const-eval) cc6f37f std: fix busy-waiting in `Once::wait_force`, add more tests 6fd82f1 std: implement the `once_wait` feature 0c56873 Remove unneeded `pub(crate)` 787a1f7 Rollup merge of rust-lang#128388 - beetrees:f16-f128-slightly-improve-windows-abi, r=tgross35 e3a4ed3 Rollup merge of rust-lang#128387 - liigo:patch-14, r=tgross35 8b7f4ee refactor(pal/hermit): use default impl of `GlobalAlloc::alloc_zeroed` c337019 refactor(pal/hermit): return `!` to satisfy rust-analyzer 7aafdcf android: Remove libstd hacks for unsupported Android APIs ba65c6c Move Windows implementation of anon pipe 176508c Match LLVM ABI in `extern "C"` functions for `f128` on Windows 85e4ba0 Cleanup sys module to match house style ddff2b6 Auto merge of rust-lang#128083 - Mark-Simulacrum:bump-bootstrap, r=albertlarsan68 e4b0e6d Rewrite binary search implementation 556dc60 More detailed note to deprecate ONCE_INIT 440ec83 Auto merge of rust-lang#128378 - matthiaskrgr:rollup-i3qz9uo, r=matthiaskrgr a50fe57 Auto merge of rust-lang#128250 - Amanieu:select_unpredictable, r=nikic 47f9d61 Rollup merge of rust-lang#128315 - zetanumbers:psvita-unsafe-in-unsafe, r=workingjubilee f70ce7f Auto merge of rust-lang#128234 - jcsp:retain-empty-case, r=tgross35 93b2f7c Insert some blank lines. db0222e Move a comment. cc96f3e Stabilize offset_of_nested 618fdd5 Auto merge of rust-lang#128334 - matthiaskrgr:rollup-nhxdt0c, r=matthiaskrgr e088cb1 Rollup merge of rust-lang#128333 - RalfJung:miri-sync, r=RalfJung 1ea0493 Rollup merge of rust-lang#128307 - ojeda:unescaped_backticks, r=GuillaumeGomez 5d51099 Optimize empty case in Vec::retain f2bcbec Auto merge of rust-lang#125016 - nicholasbishop:bishop-cb-112, r=tgross35 7a43feb Rollup merge of rust-lang#128310 - kmicklas:btree-map-peek-next-docs, r=tgross35 03e5078 Rollup merge of rust-lang#128055 - workingjubilee:deny-unsafe-ops-in-sys-personality-dwarf-eh, r=Amanieu f9befad Rollup merge of rust-lang#109174 - soerenmeier:cursor_fns, r=dtolnay ed7d02f Update compiler_builtins to 0.1.114 80254cd Warn on `rustdoc::unescaped_backticks` for `core/alloc/std/test/proc_macro` c8db8ea Remove spurious backticks detected by `rustdoc::unescaped_backticks` d1d4fb3 Reformat `use` declarations. 3ec244f Replace `io::Cursor::{remaining_slice, is_empty}` with `io::Cursor::{split, split_mut}` abc611f step cfg(bootstrap) 78cd779 Update CURRENT_RUSTC_VERSION 70927dc Add forbid(unsafe_op_in_unsafe_fn) 06a22c9 Rollup merge of rust-lang#128240 - mbrubeck:patch-3, r=joboet 604d618 Rollup merge of rust-lang#128228 - slanterns:const_waker, r=dtolnay,oli-obk 2a70839 Rollup merge of rust-lang#128103 - folkertdev:unsigned-int-is-multiple-of, r=Amanieu 058f1d3 Rollup merge of rust-lang#127765 - bitfield:fix_stdlib_doc_nits, r=dtolnay 9a6c84e fix: psvita's std code 5119266 Force LLVM to use CMOV for binary search d6b6e63 stabilize const_waker 8e4f58a Add missing periods on `BTreeMap` cursor `peek_next` docs 458b9b0 Implement cursors for `BTreeSet` 02bf0de Enable `std::io::copy` specialisation for `std::pipe::{PipeReader, PipeWriter}` 1f83bf3 Rollup merge of rust-lang#128282 - pitaj:nonzero_bitwise, r=workingjubilee 357ff7a Rollup merge of rust-lang#128279 - slanterns:is_sorted, r=dtolnay fcbdcae stabilize `is_sorted` c47f8bd bitwise and bytewise methods on `NonZero` 4ea98d7 Rollup merge of rust-lang#128259 - sunshowers:msg-nosignal, r=Mark-Simulacrum d6f970a Rollup merge of rust-lang#125897 - RalfJung:from-ref, r=Amanieu 339f756 Auto merge of rust-lang#128255 - stepancheg:doc-shl, r=scottmcm 0d6a7dd Merge from rustc a66bc79 Auto merge of rust-lang#127946 - tgross35:fmt-builders-set-result, r=cuviper 2986bfe [illumos/solaris] set MSG_NOSIGNAL while writing to sockets 3e85493 Document int.checked_shl(BITS - 1) 59f3fef Rollup merge of rust-lang#128235 - harryscholes:fix-iterator-filter-docs, r=tgross35 a2dbfd3 Rollup merge of rust-lang#124941 - Skgland:stabilize-const-int-from-str, r=dtolnay 5b78bae Add links from `assert_eq!` docs to `debug_assert_eq!`, etc. a0f135d Always set `result` during `finish()` in debug builders bd11b3d Fix docs 22ce603 Auto merge of rust-lang#128165 - saethlin:optimize-clone-shims, r=compiler-errors fb7d2a8 Fix doc nits a152820 Rollup merge of rust-lang#128170 - saethlin:clone-fn, r=compiler-errors 0d63614 Merge from rustc cb8f69b Rollup merge of rust-lang#128211 - juliusl:pr/align-change-time, r=tgross35 ba0582b Rollup merge of rust-lang#128150 - BoxyUwU:std_only_sized_const_params, r=workingjubilee 30cfde4 Rollup merge of rust-lang#127950 - nnethercote:rustfmt-skip-on-use-decls, r=cuviper 8488ae6 Make Clone::clone a lang item 1342ef1 fix: compilation issue w/ refactored type 92c0ad7 Let InstCombine remove Clone shims inside Clone shims c788415 Stop using `unsized_const_parameters` in core/std ef4d4a0 Auto merge of rust-lang#128195 - matthiaskrgr:rollup-195dfdf, r=matthiaskrgr 5b6c1e1 Rollup merge of rust-lang#128137 - GrigorenkoPV:cstr-derive, r=dtolnay aaeac06 Rollup merge of rust-lang#127999 - ChrisDenton:arm32, r=Amanieu ea3a99f Rollup merge of rust-lang#128158 - workingjubilee:unsafe-wrap-personality-gcc, r=ChrisDenton 886fe5b Rollup merge of rust-lang#127300 - biabbas:fix_connect_timeout, r=tgross35 b889a1d CStr: derive PartialEq, Eq; add test for Ord 3a18110 In connect timeout, read readiness of socket for vxworks. Check pollhup or pollerr for refused connections in linux c4ee91f Merge from rustc 244d843 std: update comments on gcc personality fn d252b6b std: unsafe-wrap gcc::rust_eh_personality and impl 09bda4f Rollup merge of rust-lang#128135 - joboet:reduplicate_tls, r=tgross35 a4c88bc Rollup merge of rust-lang#128046 - GrigorenkoPV:90435, r=tgross35 2614d86 Rollup merge of rust-lang#126548 - rik86189:issue-88264-fix, r=tgross35 75a178f Rollup merge of rust-lang#126042 - davidzeng0:master, r=Amanieu 1e118ae Rollup merge of rust-lang#128131 - ChrisDenton:stuff, r=workingjubilee cea2ca9 Rollup merge of rust-lang#128120 - compiler-errors:async-fn-name, r=oli-obk 92e3688 Rollup merge of rust-lang#127733 - GrigorenkoPV:don't-forget, r=Amanieu 9b1cffd Rollup merge of rust-lang#127480 - biabbas:vxworks, r=workingjubilee 2632261 Rollup merge of rust-lang#127252 - fitzgen:edge-cases-for-bitwise-operations, r=m-ou-se b0d7414 Rollup merge of rust-lang#126152 - RalfJung:size_of_val_raw, r=saethlin 6d0b714 Improved clarity of documentation for std::fs::create_dir_all eb79e09 std: use duplicate thread local state in tests 8456a97 Forbid unsafe_op_in_unsafe_fn in sys/pal/windows 5a9fb1f Import `core::ffi::c_void` in more places 16450f7 Merge from rustc 697c717 Add chroot unsupported implementation for VxWorks 6242470 Rollup merge of rust-lang#128106 - hallfox:patch-1, r=ChrisDenton de086ea Rollup merge of rust-lang#128092 - ChrisDenton:wrappers, r=workingjubilee ccba33c Rollup merge of rust-lang#128043 - safinaskar:primitive, r=workingjubilee a069998 Rollup merge of rust-lang#127481 - a1phyr:pattern_gat, r=Amanieu bc7345c Rollup merge of rust-lang#126770 - wr7:master, r=Amanieu 21f6b65 Rollup merge of rust-lang#125962 - Coekjan:const-binary-heap, r=Amanieu eee5bba Auto merge of rust-lang#127153 - NobodyXu:pipe, r=ChrisDenton fd3a45f Gate AsyncFn* under async_closure feature 0374ea2 Add elem_offset and related methods 2b3eacb library/core/src/primitive.rs: small doc fix 45f80e6 Fix return type of FileAttr methods on AIX target 6cabb65 add `is_multiple_of` for unsigned integer types 4c4a93a Initial implementation of anonymous_pipe 63d2997 Update process vxworks, set default stack size of 256 Kib for vxworks. User can set the stack size using RUST_MIN_STACK, with min size of libc::PTHREAD_STACK_MIN(4kib) ee86041 Rollup merge of rust-lang#128089 - workingjubilee:commonly-wrapped-to-make-safe, r=ChrisDenton ad9a52d Rollup merge of rust-lang#125834 - workingjubilee:weaken-thir-unsafeck-for-addr-of-static-mut, r=compiler-errors 642c69b Remove wrapper functions from c.rs de2a037 std: Unsafe-wrap backtrace code held in-common 155aef9 std: Unsafe-wrap alloc code held in-common bee0155 Cfg disable on_broken_pipe_flag_used() for vxworks 227b5af Disable dirfd for vxworks, Return unsupported error from set_times and lchown for vxworks 22a6797 Allow unused unsafe for vxworks in read_at and write at 8be45a9 Docs for core::primitive: mention that "core" can be shadowed, too, so we should write "::core" c039ee8 library: vary unsafety in bootstrapping for SEH 6765b97 std: unsafe-wrap personality::dwarf::eh 7ae76f0 Rollup merge of rust-lang#128008 - weiznich:fix/121521, r=lcnr 323e962 Rollup merge of rust-lang#127996 - ian-h-chamberlain:fix/horizon-warnings-unsafe-in-unsafe, r=tgross35 ae6187f Rollup merge of rust-lang#127415 - AljoschaMeyer:master, r=dtolnay d6a36f5 Use given allocator instad of Global d0bc9a0 Start using `#[diagnostic::do_not_recommend]` in the standard library ba43261 Rollup merge of rust-lang#127583 - Nilstrieb:invalid-utf8, r=joboet 0727e53 Fix warnings when checking armv6k-nintendo-3ds bbe4da8 Fix some `#[cfg_attr(not(doc), repr(..))]` 321dbf8 Deal with invalid UTF-8 from `gai_strerror` 6aa00e1 std::thread: available_parallelism implementation for vxWorks proposal. 2fff48d Auto merge of rust-lang#127722 - BoxyUwU:new_adt_const_params_limitations, r=compiler-errors d7770e9 Rollup merge of rust-lang#128005 - ChrisDenton:msvc-include, r=joboet 8fdee23 Rollup merge of rust-lang#127734 - ChrisDenton:netc, r=Mark-Simulacrum 3b2536e Remove _tls_used hack 07dbb38 Rollup merge of rust-lang#127873 - workingjubilee:forbid-unsafe-ops-for-kmc-solid, r=Amanieu bd26295 Rollup merge of rust-lang#127843 - workingjubilee:break-up-big-ass-stack-overflow-fn, r=joboet 00b4f61 Inject win arm32 shims into metadata generation 2b62867 Rollup merge of rust-lang#127918 - ChrisDenton:thread-name-string, r=joboet a077eb1 Rollup merge of rust-lang#123196 - Ayush1325:uefi-process, r=joboet eb09be4 std: forbid unwrapped unsafe in unsupported_backslash dcb9854 kmc-solid: forbid(unsafe_op_in_unsafe_fn) 845a2f7 Auto merge of rust-lang#127982 - matthiaskrgr:rollup-nzyvphj, r=matthiaskrgr 00d6fc4 Rollup merge of rust-lang#127978 - nyurik:lib-refs, r=workingjubilee 4d8afcd Avoid ref when using format! for perf 9f20a0f Rollup merge of rust-lang#126199 - ivan-shrimp:nonzero_isqrt, r=tgross35 f06530c Rollup merge of rust-lang#112328 - juliusl:pr/windows-add-change-time, r=ChrisDenton 8d5cf50 uefi: process: Fixes from PR c6cb67c uefi: process: Final Touchups afe1ef0 uefi: process: Add CommandArgs support ef6b173 uefi: process: Add support for args 1991fe3 uefi: process Implement inherit 24a9582 uefi: process: Add null protocol 36a0e1e uefi: process: Add stderr support b712e74 uefi: process: Add support to capture stdout e6eeb4e uefi: Add process f3b1c8a improve safety comment 9348998 add `NonZero<uN>::isqrt` edc4cdc Use `#[rustfmt::skip]` on some `use` groups to prevent reordering. 489f1ef unix: acquire-load NEED_ALTSTACK 9e11e01 unix: Unsafe-wrap stack_overflow::{drop,make}_handler 72c7444 unix: Unsafe-wrap stack_overflow::cleanup 33a32f2 unix: lift init of sigaltstack before sigaction 9fb6e49 unix: Unsafe-wrap stack_overflow::signal_handler c99ebd4 Rollup merge of rust-lang#127594 - c6c7:fuchsia-status-code-match-arm, r=tmandry 8378261 Move ThreadName conversions to &cstr/&str 68e2391 Style change 16bce8a Make `Thread::new_inner` a safe function d1d9893 Rollup merge of rust-lang#127748 - scottmcm:option_len, r=joboet b0c85ba Rollup merge of rust-lang#124881 - Sp00ph:reentrant_lock_tid, r=joboet 7e21850 Update `ReentrantLock` implementation, add `CURRENT_ID` thread local. c10a929 Safely enforce thread name requirements cc4ed95 Rollup merge of rust-lang#127077 - tbu-:pr_doc_fd_to_owned, r=workingjubilee 37d7bff Rollup merge of rust-lang#127861 - Kriskras99:patch-1, r=tgross35 3d50720 Rollup merge of rust-lang#127859 - RalfJung:ptr-dyn-metadata, r=scottmcm 1f3311b Rollup merge of rust-lang#127845 - workingjubilee:actually-break-up-big-ass-stack-overflow-fn, r=joboet 5578593 Auto merge of rust-lang#127865 - matthiaskrgr:rollup-8m49dlg, r=matthiaskrgr 54728b1 feat: adding ext that returns change_time for Windows b164bab Auto merge of rust-lang#125942 - timokroeger:windows-once-futex, r=ChrisDenton 0eda3a3 Rollup merge of rust-lang#127337 - celinval:intrinsics-fallback, r=oli-obk ed3c6d1 Mention how you can go from `BorrowedFd` to `OwnedFd` and back 455bd57 Make language around `ToOwned` for `BorrowedFd` more precise ab7a0d4 Document the column numbers for the dbg! macro 89cd225 ptr::metadata: update comment on vtable_ptr work-around 51e54a4 ptr::metadata: avoid references to extern types be0c06b Split part of `adt_const_params` into `unsized_const_params` 857ed93 Forbid `!Sized` types and references aedc16c unix: unsafe-wrap install_main_guard_default 4db3aa1 unix: clean up install_main_guard_freebsd d167f00 unix: stack_start_aligned is a safe fn 27b79e6 unix: split stack_overflow::install_main_guard by os d50143f Update name of Windows abort constant to match platform documentation cbaa831 Add match arm for Fuchsia status code upon an abort in a test f15715f lib: replace some `mem::forget`'s with `ManuallyDrop` 9bbf09d Windows: move BSD socket shims to netc d76c965 Remove generic lifetime parameter of trait `Pattern` ad3db57 Use Option's discriminant as its size hint 3b86ae3 Explicitly ignore `into_raw_handle()` using `let _ =` in sys/pal/windows. 1cb5354 Add `must_use` to IntoRawFd/IntoRawSocket/IntoRawHandle's methods. 697377a Clarify/add `must_use` messages for more `into_raw*` functions of `alloc` types. a7bec56 size_of_val_raw: for length 0 this is safe to call 39c4daa Reset sigpipe not supported for vxworks ca537d2 Fix them doc examples some more be23cef Fix doc examples 8d01996 Run formatter on alloc/src/boxed.rs c3b602a Add missing try_new_uninit_slice_in and try_new_zeroed_slice_in 5487574 Document safety of a few intrinsics 87fcd2f Move a few intrinsics to use Rust abi 6b549ba mark `can_not_overflow` as `#[rustc_const_stable(...)]` 259c058 stabilize `const_int_from_str` 538fe81 Add edge-case examples to `{count,leading,trailing}_{ones,zeros}` methods 5fc66dd Implement `unsigned_signed_diff` dbbb4ab less garbage, more examples 4952644 update tracking issue for `const_binary_heap_new_in` 33389b0 more explicitly state the basic rules of working with the obtained raw pointers 395ad9f Windows: Use futex implementation for `Once` 7953644 from_ref, from_mut: clarify domain of quantification git-subtree-dir: library git-subtree-split: 9cc3bc6

Fixes rust-lang#130210. Since rust-lang#124881, `ReentrantLock` uses `ThreadId` to identify threads. This has the unfortunate consequence of breaking uses of `Stdout` before main: Locking the `ReentrantLock` that synchronizes the output will initialize the thread ID before the handle for the main thread is set in `rt::init`. But since that would overwrite the current thread ID, `thread::set_current` triggers an abort. This PR fixes the problem by using the already initialized thread ID for constructing the main thread handle and allowing `set_current` calls that do not change the thread's ID.

std: fix stdout-before-main Fixes rust-lang#130210. Since rust-lang#124881, `ReentrantLock` uses `ThreadId` to identify threads. This has the unfortunate consequence of breaking uses of `Stdout` before main: Locking the `ReentrantLock` that synchronizes the output will initialize the thread ID before the handle for the main thread is set in `rt::init`. But since that would overwrite the current thread ID, `thread::set_current` triggers an abort. This PR fixes the problem by using the already initialized thread ID for constructing the main thread handle and allowing `set_current` calls that do not change the thread's ID.

Rollup merge of rust-lang#131233 - joboet:stdout-before-main, r=tgross35 std: fix stdout-before-main Fixes rust-lang#130210. Since rust-lang#124881, `ReentrantLock` uses `ThreadId` to identify threads. This has the unfortunate consequence of breaking uses of `Stdout` before main: Locking the `ReentrantLock` that synchronizes the output will initialize the thread ID before the handle for the main thread is set in `rt::init`. But since that would overwrite the current thread ID, `thread::set_current` triggers an abort. This PR fixes the problem by using the already initialized thread ID for constructing the main thread handle and allowing `set_current` calls that do not change the thread's ID.

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels May 8, 2024

RalfJung reviewed May 8, 2024

View reviewed changes

joboet requested changes May 8, 2024

View reviewed changes

rustbot added I-libs-api-nominated Nominated for discussion during a libs-api team meeting. labels May 8, 2024

joboet added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels May 8, 2024

m-ou-se removed the I-libs-api-nominated Nominated for discussion during a libs-api team meeting. label May 21, 2024

m-ou-se self-assigned this May 21, 2024

rust-cloud-vms bot force-pushed the reentrant_lock_tid branch from 4d29740 to 26e800d Compare May 25, 2024 01:58

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels May 25, 2024

CAD97 mentioned this pull request Jun 4, 2024

thread_id_value tracking issue #67939

Open

joboet approved these changes Jul 17, 2024

View reviewed changes

rustbot assigned joboet and unassigned m-ou-se Jul 17, 2024

joboet mentioned this pull request Jul 17, 2024

Use atomics instead of mutex in exit guard #127863

Closed

rust-cloud-vms bot force-pushed the reentrant_lock_tid branch 2 times, most recently from ebb48a1 to 2296ffb Compare July 18, 2024 00:12

joboet approved these changes Jul 18, 2024

View reviewed changes

rust-cloud-vms bot force-pushed the reentrant_lock_tid branch from 5bd459a to fe89962 Compare July 18, 2024 14:12

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 18, 2024

matthiaskrgr mentioned this pull request Jul 18, 2024

Rollup of 6 pull requests #127924

Merged

bors merged commit f62aa41 into rust-lang:master Jul 18, 2024
6 checks passed

rustbot added this to the 1.81.0 milestone Jul 18, 2024

joboet mentioned this pull request Jul 20, 2024

Tracking issue for cleaning up std's thread_local implementation details #110897

Open

24 tasks

ehuss mentioned this pull request Sep 10, 2024

cargo test for an LD_PRELOAD dylib causes runtime error in 1.81.0 #130210

Closed

joboet mentioned this pull request Oct 4, 2024

std: fix stdout-before-main #131233

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use ThreadId instead of TLS-address in `ReentrantLock` #124881

Use ThreadId instead of TLS-address in `ReentrantLock` #124881

Sp00ph commented May 8, 2024

RalfJung commented May 8, 2024

RalfJung May 8, 2024

RalfJung May 8, 2024

Sp00ph May 8, 2024

RalfJung May 8, 2024

Sp00ph May 8, 2024

RalfJung May 8, 2024

Sp00ph May 8, 2024

RalfJung May 8, 2024

Sp00ph May 8, 2024

RalfJung May 8, 2024

Sp00ph May 8, 2024

RalfJung May 8, 2024 •

edited

Loading

Sp00ph May 8, 2024

joboet left a comment

joboet May 8, 2024

joboet May 8, 2024

Sp00ph commented May 25, 2024

Sp00ph commented Jul 16, 2024

joboet left a comment

joboet Jul 17, 2024

joboet Jul 17, 2024

joboet Jul 17, 2024

joboet Jul 17, 2024 •

edited

Loading

Sp00ph commented Jul 18, 2024

joboet left a comment

joboet Jul 18, 2024

joboet Jul 18, 2024

joboet Jul 18, 2024

joboet commented Jul 18, 2024

bors commented Jul 18, 2024

Sp00ph commented Jul 18, 2024

bors commented Jul 18, 2024

	crate::thread::try_current_id().map_or_else(\|\| no_tid(), \|tid\| tid.as_u64().get())
	crate::thread::try_current_id().unwrap_or_else(\|\| no_tid()).as_u64().get()

	const fn new(tid: u64) -> Self {
	const fn new(tid: Option<ThreadId>) -> Self {

Use ThreadId instead of TLS-address in ReentrantLock #124881

Use ThreadId instead of TLS-address in ReentrantLock #124881

Conversation

Sp00ph commented May 8, 2024

RalfJung commented May 8, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

RalfJung May 8, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

joboet left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Sp00ph commented May 25, 2024

Sp00ph commented Jul 16, 2024

joboet left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

joboet Jul 17, 2024 • edited Loading

Choose a reason for hiding this comment

Sp00ph commented Jul 18, 2024

joboet left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

joboet commented Jul 18, 2024

bors commented Jul 18, 2024

Sp00ph commented Jul 18, 2024

bors commented Jul 18, 2024

Use ThreadId instead of TLS-address in `ReentrantLock` #124881

Use ThreadId instead of TLS-address in `ReentrantLock` #124881

RalfJung May 8, 2024 •

edited

Loading

joboet Jul 17, 2024 •

edited

Loading