Skip to content

Conversation

@joboet
Copy link
Member

@joboet joboet commented Nov 30, 2025

Fixes rust-lang/miri#4737

#144465 changed ThreadId::new to use a spinlock on platforms without 64-bit atomics. This introduces subtle scheduling changes such as the one described in rust-lang/miri#4737 and isn't ideal from a performance perspective either. The necessity of avoiding Mutex is not always given however – it only needs to be avoided if the thread handle is lazily initialised. For normal Rust threads however, std generates the thread id inside the spawning logic, where allocation is expected. Thus this PR renames the current ThreadId::new to desperate_new and reintroduces a version ThreadId::new that employs a Mutex to synchronise access to the inner spin-lock.

CC @RalfJung @orlp

@rustbot 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 Nov 30, 2025
@rustbot
Copy link
Collaborator

rustbot commented Nov 30, 2025

r? @Mark-Simulacrum

rustbot has assigned @Mark-Simulacrum.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

#[cold]
fn exhausted() -> ! {
panic!("failed to generate unique thread ID: bitspace exhausted")
rtabort!("failed to generate unique thread ID: bitspace exhausted")
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Panicking may allocate, so it shouldn't be used here either.

@orlp
Copy link
Contributor

orlp commented Nov 30, 2025

This violates the contract we established that the global allocator may call current_thread() which requires the thread ID to be initialized. But if Mutex allocates (which it does on some platforms), you've now got a circular dependency.

So if there's a platform without 64-bit atomics but which has an allocating Mutex this will blow up I believe.

@joboet
Copy link
Member Author

joboet commented Nov 30, 2025

This violates the contract we established that the global allocator may call current_thread() which requires the thread ID to be initialized. But if Mutex allocates (which it does on some platforms), you've now got a circular dependency.

So if there's a platform without 64-bit atomics but which has an allocating Mutex this will blow up I believe.

No it won't, the Mutex isn't used from ThreadId::desperate_new, which is what is called from current_thread().

@orlp
Copy link
Contributor

orlp commented Nov 30, 2025

@joboet Actually, re-reading the code, neither ThreadId::new or ThreadId::desperate_new should be getting called from current_thread() during normal operation because I set the current ID before anything in ThreadInit::init. Only if it is called in the really strange thread cleanup state.

If we were to adopt #147342 I think we might be able to get rid of the desperate path altogether.

@joboet
Copy link
Member Author

joboet commented Nov 30, 2025

@joboet Actually, re-reading the code, neither ThreadId::new or ThreadId::desperate_new should be getting called from current_thread() during normal operation because I set the current ID before anything in ThreadInit::init. Only if it is called in the really strange thread cleanup state.

The problem is that not all threads are spawned by Rust – and on foreign threads, it's the first call to thread::current that initialises the id, not ThreadInit::init.

@orlp
Copy link
Contributor

orlp commented Nov 30, 2025

I hadn't considered foreign threads, you're right.

@rust-log-analyzer
Copy link
Collaborator

The job x86_64-gnu-miri failed! Check out the build log: (web) (plain enhanced) (plain)

Click to see the possible cause of the failure (guessed by this bot)
tests/fail-dep/libc/fs/unix_open_missing_required_mode.rs ... ok
tests/fail-dep/libc/fs/write_to_stdin.rs ... ok

FAILED TEST: tests/fail-dep/concurrency/libc_pthread_join_multiple.rs
command: MIRI_ENV_VAR_TEST="0" MIRI_TEMP="/tmp/miri-uitest-cCF470" RUST_BACKTRACE="1" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/bin/miri" "--error-format=json" "--sysroot=/checkout/obj/build/x86_64-unknown-linux-gnu/miri-sysroot" "-Dwarnings" "-Dunused" "-Ainternal_features" "-Zui-testing" "--out-dir" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2-tools/x86_64-unknown-linux-gnu/tmp/miri_ui/0/tests/fail-dep/concurrency" "tests/fail-dep/concurrency/libc_pthread_join_multiple.rs" "--extern" "cfg_if=/checkout/obj/build/x86_64-unknown-linux-gnu/stage2-tools/aarch64-apple-darwin/debug/deps/libcfg_if-94b97c8c638a9a0b.rlib" "--extern" "cfg_if=/checkout/obj/build/x86_64-unknown-linux-gnu/stage2-tools/aarch64-apple-darwin/debug/deps/libcfg_if-94b97c8c638a9a0b.rmeta" "--extern" "getrandom_01=/checkout/obj/build/x86_64-unknown-linux-gnu/stage2-tools/aarch64-apple-darwin/debug/deps/libgetrandom-4f5d24ccf6a2e71f.rlib" "--extern" "getrandom_01=/checkout/obj/build/x86_64-unknown-linux-gnu/stage2-tools/aarch64-apple-darwin/debug/deps/libgetrandom-4f5d24ccf6a2e71f.rmeta" "--extern" "getrandom_02=/checkout/obj/build/x86_64-unknown-linux-gnu/stage2-tools/aarch64-apple-darwin/debug/deps/libgetrandom-d252fe7f74cfc69c.rlib" "--extern" "getrandom_02=/checkout/obj/build/x86_64-unknown-linux-gnu/stage2-tools/aarch64-apple-darwin/debug/deps/libgetrandom-d252fe7f74cfc69c.rmeta" "--extern" "getrandom_03=/checkout/obj/build/x86_64-unknown-linux-gnu/stage2-tools/aarch64-apple-darwin/debug/deps/libgetrandom-a46b8268546a5043.rlib" "--extern" "getrandom_03=/checkout/obj/build/x86_64-unknown-linux-gnu/stage2-tools/aarch64-apple-darwin/debug/deps/libgetrandom-a46b8268546a5043.rmeta" "--extern" "libc=/checkout/obj/build/x86_64-unknown-linux-gnu/stage2-tools/aarch64-apple-darwin/debug/deps/liblibc-c19e0007c2efab56.rlib" "--extern" "libc=/checkout/obj/build/x86_64-unknown-linux-gnu/stage2-tools/aarch64-apple-darwin/debug/deps/liblibc-c19e0007c2efab56.rmeta" "--extern" "num_cpus=/checkout/obj/build/x86_64-unknown-linux-gnu/stage2-tools/aarch64-apple-darwin/debug/deps/libnum_cpus-3ac91b21e8612173.rlib" "--extern" "num_cpus=/checkout/obj/build/x86_64-unknown-linux-gnu/stage2-tools/aarch64-apple-darwin/debug/deps/libnum_cpus-3ac91b21e8612173.rmeta" "--extern" "futures=/checkout/obj/build/x86_64-unknown-linux-gnu/stage2-tools/aarch64-apple-darwin/debug/deps/libfutures-1a02ebb05fa6d2b2.rlib" "--extern" "futures=/checkout/obj/build/x86_64-unknown-linux-gnu/stage2-tools/aarch64-apple-darwin/debug/deps/libfutures-1a02ebb05fa6d2b2.rmeta" "--extern" "page_size=/checkout/obj/build/x86_64-unknown-linux-gnu/stage2-tools/aarch64-apple-darwin/debug/deps/libpage_size-f53fa59f02c6bf4f.rlib" "--extern" "page_size=/checkout/obj/build/x86_64-unknown-linux-gnu/stage2-tools/aarch64-apple-darwin/debug/deps/libpage_size-f53fa59f02c6bf4f.rmeta" "--extern" "tempfile=/checkout/obj/build/x86_64-unknown-linux-gnu/stage2-tools/aarch64-apple-darwin/debug/deps/libtempfile-baf9917319ac0e00.rlib" "--extern" "tempfile=/checkout/obj/build/x86_64-unknown-linux-gnu/stage2-tools/aarch64-apple-darwin/debug/deps/libtempfile-baf9917319ac0e00.rmeta" "--extern" "tokio=/checkout/obj/build/x86_64-unknown-linux-gnu/stage2-tools/aarch64-apple-darwin/debug/deps/libtokio-7f81a70465c0e732.rlib" "--extern" "tokio=/checkout/obj/build/x86_64-unknown-linux-gnu/stage2-tools/aarch64-apple-darwin/debug/deps/libtokio-7f81a70465c0e732.rmeta" "-L" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2-tools/debug/deps" "-L" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2-tools/aarch64-apple-darwin/debug/deps" "--edition" "2021" "--target" "aarch64-apple-darwin"

error: actual output differed from expected
Execute `./miri test --bless` to update `tests/fail-dep/concurrency/libc_pthread_join_multiple.stderr` to the actual output
--- tests/fail-dep/concurrency/libc_pthread_join_multiple.stderr
+++ <stderr output>
 error: Undefined Behavior: trying to join an already joined thread
   --> tests/fail-dep/concurrency/libc_pthread_join_multiple.rs:LL:CC
    |
-LL | ...   assert_eq!(libc::pthread_join(native_copy, ptr::null_mut()), 0);
+LL |         assert_eq!(libc::pthread_join(native, ptr::null_mut()), 0);
-   |                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Undefined Behavior occurred here
+   |                    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Undefined Behavior occurred here
    |
    = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
... 4 lines skipped ...
 error: aborting due to 1 previous error
 

Full unnormalized output:
error: Undefined Behavior: trying to join an already joined thread
##[error]  --> tests/fail-dep/concurrency/libc_pthread_join_multiple.rs:26:20
   |
LL |         assert_eq!(libc::pthread_join(native, ptr::null_mut()), 0);
   |                    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Undefined Behavior occurred here
   |
   = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
   = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information

note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace
---

error: there was 1 unmatched diagnostic
##[error]  --> tests/fail-dep/concurrency/libc_pthread_join_multiple.rs:26:20
   |
26 |         assert_eq!(libc::pthread_join(native, ptr::null_mut()), 0);
   |                    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Error: Undefined Behavior: trying to join an already joined thread
   |

full stderr:
error: Undefined Behavior: trying to join an already joined thread
##[error]  --> tests/fail-dep/concurrency/libc_pthread_join_multiple.rs:26:20
   |
LL |         assert_eq!(libc::pthread_join(native, ptr::null_mut()), 0);
   |                    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Undefined Behavior occurred here
   |
   = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
   = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information

note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace
---
Location:
   /cargo/registry/src/index.crates.io-1949cf8c6b5b557f/ui_test-0.30.3/src/lib.rs:365

  ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ BACKTRACE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
   1: <color_eyre[3372d94b51e7a708]::config::EyreHook>::into_eyre_hook::{closure#0}<unknown>
      at <unknown source file>:<unknown line>
   2: eyre[6a79be49b3fe138]::private::format_err<unknown>
      at <unknown source file>:<unknown line>
   3: ui_test[d668d2b9b1b3f14d]::run_tests_generic::<ui_test[d668d2b9b1b3f14d]::default_file_filter, ui[fc92290033120f0c]::run_tests::{closure#1}, alloc[d38f3209116a0e5b]::boxed::Box<dyn ui_test[d668d2b9b1b3f14d]::status_emitter::StatusEmitter>><unknown>
      at <unknown source file>:<unknown line>
   4: ui[fc92290033120f0c]::ui<unknown>
      at <unknown source file>:<unknown line>
   5: ui[fc92290033120f0c]::main<unknown>
      at <unknown source file>:<unknown line>
   6: std[ab77aa9e69dcd316]::sys::backtrace::__rust_begin_short_backtrace::<fn() -> core[15d2f2e4ef059d4d]::result::Result<(), eyre[6a79be49b3fe138]::Report>, core[15d2f2e4ef059d4d]::result::Result<(), eyre[6a79be49b3fe138]::Report>><unknown>
      at <unknown source file>:<unknown line>
   7: std[ab77aa9e69dcd316]::rt::lang_start::<core[15d2f2e4ef059d4d]::result::Result<(), eyre[6a79be49b3fe138]::Report>>::{closure#0}<unknown>
      at <unknown source file>:<unknown line>
   8: std[ab77aa9e69dcd316]::rt::lang_start_internal<unknown>
      at <unknown source file>:<unknown line>
   9: main<unknown>
      at <unknown source file>:<unknown line>
  10: __libc_start_main<unknown>
      at <unknown source file>:<unknown line>
  11: _start<unknown>
      at <unknown source file>:<unknown line>

Run with COLORBT_SHOW_HIDDEN=1 environment variable to disable frame filtering.
Run with RUST_BACKTRACE=full to include source snippets.
error: test failed, to rerun pass `--test ui`

Caused by:
  process didn't exit successfully: `/checkout/obj/build/x86_64-unknown-linux-gnu/stage2-tools/x86_64-unknown-linux-gnu/release/deps/ui-e3df02d956e0ccf4` (exit status: 1)
Command `/checkout/obj/build/x86_64-unknown-linux-gnu/stage0/bin/cargo test --target x86_64-unknown-linux-gnu -Zbinary-dep-depinfo -j 4 -Zroot-dir=/checkout --locked --color always --release --manifest-path /checkout/src/tools/miri/Cargo.toml -- [workdir=/checkout]` failed with exit code 1
Created at: src/bootstrap/src/core/build_steps/tool.rs:191:21
Executed at: src/bootstrap/src/core/build_steps/test.rs:690:19

Command has failed. Rerun with -v to see more details.
Bootstrap failed while executing `test --stage 2 src/tools/miri src/tools/miri/cargo-miri --target aarch64-apple-darwin`
Build completed unsuccessfully in 0:02:56
  local time: Sun Nov 30 11:54:21 UTC 2025
  network time: Sun, 30 Nov 2025 11:54:21 GMT
##[error]Process completed with exit code 1.

@RalfJung
Copy link
Member

This still invokes compare_exchange_weak in normal std operation, doesn't it? So it won't actually help for Miri.

}
}

/// Generates a new unique thread ID.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
/// Generates a new unique thread ID.
/// Generates a new unique thread ID. Might use the global allocator.

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Nov 30, 2025
ThreadId generation fallback path: avoid spurious yields

Fixes rust-lang/miri#4737
Alternative to rust-lang#149476
Cc `@orlp` `@joboet`
bors added a commit that referenced this pull request Nov 30, 2025
ThreadId generation fallback path: avoid spurious yields

Fixes rust-lang/miri#4737
Alternative to #149476
Cc `@orlp` `@joboet`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Unexpected non-determinism with thread::scope (but only on some targets)

6 participants