Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Use the OS thread name by default if THREAD_INFO has not been initialized #121666

Merged
merged 3 commits into from
Mar 2, 2024
Merged

Use the OS thread name by default if THREAD_INFO has not been initialized #121666

merged 3 commits into from
Mar 2, 2024

Conversation

ChrisDenton
Copy link
Member

@ChrisDenton ChrisDenton commented Feb 27, 2024 •
edited
Loading

Currently if THREAD_INFO hasn't been initialized then the name will be set to None. This PR changes it to use the OS thread name by default. This mostly affects foreign threads at the moment but we could expand this to make more use of the OS thread name in the future.

Note: I've only implemented Thread::get_name for windows, linux and macos (and macos adjacent) targets. The rest just return None.

@rustbot rustbot added O-unix Operating system: Unix-like O-windows Operating system: Windows 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 Feb 27, 2024
@ChrisDenton ChrisDenton changed the title Use the OS thread name by default if ThreadInfo has not been initialized Use the OS thread name by default if THREAD_INFO has not been initialized Feb 27, 2024
@rustbot rustbot added O-hermit Operating System: Hermit O-itron Operating System: ITRON O-SGX Target: SGX O-wasi Operating system: Wasi, Webassembly System Interface labels Mar 1, 2024
@cuviper
Copy link
Member

cuviper commented Mar 1, 2024

@bors r+

@bors
Copy link
Contributor

bors commented Mar 1, 2024

📌 Commit 6cb0c40 has been approved by cuviper

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 Mar 1, 2024
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Mar 2, 2024
@matthiaskrgr
Rollup merge of rust-lang#121666 - ChrisDenton:thread-name, r=cuviper
1af11b1 
Use the OS thread name by default if `THREAD_INFO` has not been initialized

Currently if `THREAD_INFO` hasn't been initialized then the name will be set to `None`.  This PR changes it to use the OS thread name by default. This mostly affects foreign threads at the moment but we could expand this to make more use of the OS thread name in the future.

Note: I've only implemented `Thread::get_name` for windows, linux and macos (and macos adjacent) targets. The rest just return `None`.
@matthiaskrgr matthiaskrgr mentioned this pull request Mar 2, 2024
Closed
bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 2, 2024
@bors
Auto merge of rust-lang#121897 - matthiaskrgr:rollup-89tcmau, r=matth…
978b3e0 
…iaskrgr

Rollup of 6 pull requests

Successful merges:

 - rust-lang#121194 (Refactor pre-getopts command line argument handling)
 - rust-lang#121666 (Use the OS thread name by default if `THREAD_INFO` has not been initialized)
 - rust-lang#121758 (Move thread local implementation to `sys`)
 - rust-lang#121759 (attempt to further clarify addr_of docs)
 - rust-lang#121888 (style library/core/src/error.rs)
 - rust-lang#121892 (The ordinary lowering of `thir::ExprKind::Let` is unreachable)

r? `@ghost`
`@rustbot` modify labels: rollup
@matthiaskrgr matthiaskrgr mentioned this pull request Mar 2, 2024
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 2, 2024
@bors
Auto merge of rust-lang#121904 - matthiaskrgr:rollup-qcq0d3h, r=matth…
d25ab9c 
…iaskrgr

Rollup of 7 pull requests

Successful merges:

 - rust-lang#121666 (Use the OS thread name by default if `THREAD_INFO` has not been initialized)
 - rust-lang#121758 (Move thread local implementation to `sys`)
 - rust-lang#121759 (attempt to further clarify addr_of docs)
 - rust-lang#121855 (Correctly generate item info of trait items)
 - rust-lang#121888 (style library/core/src/error.rs)
 - rust-lang#121892 (The ordinary lowering of `thir::ExprKind::Let` is unreachable)
 - rust-lang#121895 (avoid collecting into vecs in some places)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 2, 2024
@bors
Auto merge of rust-lang#121904 - matthiaskrgr:rollup-qcq0d3h, r=matth…
5119208 
…iaskrgr

Rollup of 7 pull requests

Successful merges:

 - rust-lang#121666 (Use the OS thread name by default if `THREAD_INFO` has not been initialized)
 - rust-lang#121758 (Move thread local implementation to `sys`)
 - rust-lang#121759 (attempt to further clarify addr_of docs)
 - rust-lang#121855 (Correctly generate item info of trait items)
 - rust-lang#121888 (style library/core/src/error.rs)
 - rust-lang#121892 (The ordinary lowering of `thir::ExprKind::Let` is unreachable)
 - rust-lang#121895 (avoid collecting into vecs in some places)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 0f544f2 into rust-lang:master Mar 2, 2024
11 checks passed
@rustbot rustbot added this to the 1.78.0 milestone Mar 2, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Mar 2, 2024
@rust-timer
Unrolled build for rust-lang#121666
1dff4bc 
Rollup merge of rust-lang#121666 - ChrisDenton:thread-name, r=cuviper

Use the OS thread name by default if `THREAD_INFO` has not been initialized

Currently if `THREAD_INFO` hasn't been initialized then the name will be set to `None`.  This PR changes it to use the OS thread name by default. This mostly affects foreign threads at the moment but we could expand this to make more use of the OS thread name in the future.

Note: I've only implemented `Thread::get_name` for windows, linux and macos (and macos adjacent) targets. The rest just return `None`.
@ChrisDenton ChrisDenton deleted the thread-name branch March 3, 2024 01:52
RalfJung
RalfJung reviewed Mar 3, 2024
View reviewed changes
library/std/src/thread/tests.rs
Thread::set_name(name);
assert_eq!(name, Thread::get_name().unwrap().as_c_str());
});
handler.join().unwrap();
Copy link
Member

Choose a reason for hiding this comment

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

Is there a particular reason why this test runs in a new thread, rather than the main thread?

Copy link
Member Author

Choose a reason for hiding this comment

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

I was worried about affecting the thread name for all tests.

Copy link
Member

Choose a reason for hiding this comment

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

Ah, makes sense. A comment explaining that would have avoided some surprise and confusion here. :)

taiki-e added a commit to taiki-e/rust-cross-toolchain that referenced this pull request Mar 3, 2024
@taiki-e
linux-musl: Add patch to fix build error on hexagon
f4527d8 
rust-lang/rust#121666 caused regression on hexagon linux-musl:

```
 ld.lld: error: undefined symbol: pthread_getname_np
```
taiki-e added a commit to taiki-e/atomic-maybe-uninit that referenced this pull request Mar 3, 2024
@taiki-e
ci: Pin nightly toolchain for hexagon test
73a786f 
rust-lang/rust#121666 caused regression on hexagon linux-musl:

```
 ld.lld: error: undefined symbol: pthread_getname_np
```
@ehuss ehuss mentioned this pull request Mar 4, 2024
Closed
@joboet joboet mentioned this pull request Apr 5, 2024
Closed
ChrisDenton added a commit to ChrisDenton/rust that referenced this pull request Apr 5, 2024
@ChrisDenton
Revert rust-lang#121666
7d00826 
This reverts rust-lang#121666 due to rust-lang#123495
@ChrisDenton ChrisDenton mentioned this pull request Apr 5, 2024
Merged
workingjubilee added a commit to workingjubilee/rustc that referenced this pull request Apr 5, 2024
@workingjubilee
Rollup merge of rust-lang#123505 - ChrisDenton:revert-121666, r=worki…
7b5a6d8 
…ngjubilee

Revert "Use OS thread name by default"

This reverts rust-lang#121666 (Use the OS thread name by default if `THREAD_INFO` has not been initialized) due to rust-lang#123495 (Thread names are not always valid UTF-8).

It's not a direct revert because there have been other changes since that PR.
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request Apr 5, 2024
@GuillaumeGomez
Rollup merge of rust-lang#123505 - ChrisDenton:revert-121666, r=worki…
1bffe75 
…ngjubilee

Revert "Use OS thread name by default"

This reverts rust-lang#121666 (Use the OS thread name by default if `THREAD_INFO` has not been initialized) due to rust-lang#123495 (Thread names are not always valid UTF-8).

It's not a direct revert because there have been other changes since that PR.
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Apr 6, 2024
@rust-timer
Unrolled build for rust-lang#123505
82dbb9e 
Rollup merge of rust-lang#123505 - ChrisDenton:revert-121666, r=workingjubilee

Revert "Use OS thread name by default"

This reverts rust-lang#121666 (Use the OS thread name by default if `THREAD_INFO` has not been initialized) due to rust-lang#123495 (Thread names are not always valid UTF-8).

It's not a direct revert because there have been other changes since that PR.
ChrisDenton added a commit to ChrisDenton/rust that referenced this pull request Apr 6, 2024
@ChrisDenton
Beta revert "Use OS thread name by default"
081ad85 
This reverts rust-lang#121666 due to rust-lang#123495

This has already been done on master but beta needs something that will backport cleanly.
@ChrisDenton ChrisDenton mentioned this pull request Apr 6, 2024
Closed
cuviper pushed a commit to cuviper/rust that referenced this pull request Apr 11, 2024
@ChrisDenton @cuviper
Beta revert "Use OS thread name by default"
3e37631 
This reverts rust-lang#121666 due to rust-lang#123495

This has already been done on master but beta needs something that will backport cleanly.

(cherry picked from commit 081ad85)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ollie27 ollie27 ollie27 left review comments

@RalfJung RalfJung RalfJung left review comments

@cuviper cuviper cuviper left review comments

Assignees

@cuviper cuviper

Labels
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-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
Milestone
1.78.0
Development

Successfully merging this pull request may close these issues.
6 participants
@ChrisDenton @cuviper @bors @RalfJung @ollie27 @rustbot