Skip to content

thread name in stack overflow message #144500

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

joboet
Copy link
Member

@joboet joboet commented Jul 26, 2025

Fixes #144481, which is caused by the thread name not being initialised yet when setting up the stack overflow information. Unfortunately, the stack overflow UI test did not test for the correct thread name being present, and testing this separately didn't occur to me when writing #140628.

This PR contains the smallest possible fix I could think of: passing the thread name explicitly to the platform thread creation function. In the future I'd very much like to explore some possibilities around merging the thread packet and thread handle into one structure and using that in the platform code instead – but that's best left for another PR.

This PR also amends the stack overflow test to check for thread names, so we don't run into this again.

@rustbot label +beta-nominated

@rustbot
Copy link
Collaborator

rustbot commented Jul 26, 2025

r? @ChrisDenton

rustbot has assigned @ChrisDenton.
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

@rustbot rustbot added O-hermit Operating System: Hermit O-itron Operating System: ITRON O-SGX Target: SGX O-unix Operating system: Unix-like O-wasi Operating system: Wasi, Webassembly System Interface O-wasm Target: WASM (WebAssembly), http://webassembly.org/ O-windows Operating system: Windows S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-libs Relevant to the library team, which will review and decide on the PR/issue. beta-nominated Nominated for backporting to the compiler in the beta channel. labels Jul 26, 2025
@Urgau Urgau removed the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Jul 26, 2025
Copy link
Contributor

@benesch benesch left a comment

Choose a reason for hiding this comment

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

My review here isn't worth very much, but from what I do know this looks great. Thank you so much for fixing this so quickly, @joboet!

@ChrisDenton
Copy link
Member

This looks fair enough as a quick fix.

@bors r+

@bors
Copy link
Collaborator

bors commented Jul 26, 2025

📌 Commit dccafab has been approved by ChrisDenton

It is now in the queue for this repository.

@bors 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 26, 2025
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Jul 26, 2025
… r=ChrisDenton

thread name in stack overflow message

Fixes rust-lang#144481, which is caused by the thread name not being initialised yet when setting up the stack overflow information. Unfortunately, the stack overflow UI test did not test for the correct thread name being present, and testing this separately didn't occur to me when writing rust-lang#140628.

This PR contains the smallest possible fix I could think of: passing the thread name explicitly to the platform thread creation function. In the future I'd very much like to explore some possibilities around merging the thread packet and thread handle into one structure and using that in the platform code instead – but that's best left for another PR.

This PR also amends the stack overflow test to check for thread names, so we don't run into this again.

`@rustbot` label +beta-nominated
@ChrisDenton
Copy link
Member

@rust-lang/libs we're nearing a release so it'd be good to have a backport decision asap.

bors added a commit that referenced this pull request Jul 26, 2025
Rollup of 10 pull requests

Successful merges:

 - #144359 (add codegen test for variadics)
 - #144409 (Stop compilation early if macro expansion failed)
 - #144422 (library/windows_targets: Fix macro expansion error in 'link' macro)
 - #144430 (tests: aarch64-outline-atomics: Remove hardcoded target)
 - #144445 (Fix `./x check bootstrap` (again))
 - #144453 (canonicalize build root in `tests/run-make/linker-warning`)
 - #144454 (move uefi test to run-make)
 - #144464 (Only run bootstrap tests in `x test` on CI)
 - #144495 (bump cargo_metadata)
 - #144500 (thread name in stack overflow message)

r? `@ghost`
`@rustbot` modify labels: rollup
@matthiaskrgr
Copy link
Member

@bors r-
#144508 (comment)

@bors bors 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-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Jul 26, 2025
@benesch
Copy link
Contributor

benesch commented Jul 26, 2025

Looks like there's an alternative implementation of Thread::new for WASI that's compiled when atomics are disabled that also needs to be adjusted to take name as a parameter:

pub unsafe fn new(_stack: usize, _p: Box<dyn FnOnce()>) -> io::Result<Thread> {
crate::sys::unsupported()
}

@joboet joboet force-pushed the thread-name-stack-overflow branch from dccafab to 73751a0 Compare July 28, 2025 16:10
@joboet
Copy link
Member Author

joboet commented Jul 28, 2025

@bors r=@ChrisDenton

@bors
Copy link
Collaborator

bors commented Jul 28, 2025

📌 Commit 73751a0 has been approved by ChrisDenton

It is now in the queue for this repository.

@bors 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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jul 28, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
beta-nominated Nominated for backporting to the compiler in the beta channel. O-hermit Operating System: Hermit O-itron Operating System: ITRON O-SGX Target: SGX O-unix Operating system: Unix-like O-wasi Operating system: Wasi, Webassembly System Interface O-wasm Target: WASM (WebAssembly), http://webassembly.org/ O-windows Operating system: Windows S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. 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.

regression: thread names missing from stack overflow error messages
7 participants