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.

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

Share pthread_setname via minipal #108370

Merged
merged 2 commits into from
Oct 6, 2024
Merged

Conversation

am11
Copy link
Member

@am11 am11 commented Sep 29, 2024

Aligned coreclr, nativeaot, mono and libs.

@dotnet-policy-service dotnet-policy-service bot added the community-contribution Indicates that the PR has been added by a community member label Sep 29, 2024
@am11 am11 mentioned this pull request Sep 29, 2024
@am11 am11 force-pushed the feature/illumos-port branch 4 times, most recently from 114d062 to 9f59c12 Compare September 29, 2024 12:38
@jkotas
Copy link
Member

jkotas commented Sep 29, 2024

Should this be moved to minipal?

@am11
Copy link
Member Author

am11 commented Sep 29, 2024

Yes, it does seem like a good candidate for minipal. 😃

src/coreclr/pal/src/thread/thread.cpp Outdated Show resolved Hide resolved
src/coreclr/pal/src/thread/thread.cpp Outdated Show resolved Hide resolved
src/coreclr/pal/src/thread/thread.cpp Outdated Show resolved Hide resolved
src/coreclr/pal/src/thread/thread.cpp Outdated Show resolved Hide resolved
@am11 am11 force-pushed the feature/illumos-port branch from 6621b1f to cfd1ae2 Compare September 30, 2024 08:40
@am11 am11 changed the title Simplify pthread_setname_np usage Share pthread_setname via minipal Sep 30, 2024
@am11 am11 force-pushed the feature/illumos-port branch from 125dfc5 to 1c276fa Compare September 30, 2024 11:04
@am11 am11 marked this pull request as ready for review September 30, 2024 11:05
@am11 am11 force-pushed the feature/illumos-port branch from 1c276fa to c5927f3 Compare September 30, 2024 11:18
@am11
Copy link
Member Author

am11 commented Sep 30, 2024

cc @jkoritzinsky, a heads-up for potential conflict with your PR. 🙈

*
* @return A pointer to a null-terminated string containing the executable path,
* or NULL if an error occurs.
*/
Copy link
Member Author

Choose a reason for hiding this comment

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

aligned the documentation with utf8.h.

src/native/minipal/thread.h Outdated Show resolved Hide resolved
src/native/minipal/thread.h Outdated Show resolved Hide resolved
src/native/minipal/thread.h Outdated Show resolved Hide resolved
@am11 am11 force-pushed the feature/illumos-port branch 2 times, most recently from 5fe836b to 2f6239f Compare October 1, 2024 17:30
@am11 am11 requested a review from steveisok as a code owner October 1, 2024 17:30
@am11 am11 force-pushed the feature/illumos-port branch 2 times, most recently from 41e7fa0 to e1a0f74 Compare October 1, 2024 18:37
src/native/minipal/thread.c Outdated Show resolved Hide resolved
src/native/minipal/thread.c Outdated Show resolved Hide resolved
src/native/libs/System.Native/pal_threading.c Show resolved Hide resolved
@am11 am11 force-pushed the feature/illumos-port branch from e1a0f74 to 7096c63 Compare October 1, 2024 20:04
@am11 am11 force-pushed the feature/illumos-port branch from ba361e1 to 247d3cb Compare October 2, 2024 09:40
Copy link
Member

@jkotas jkotas left a comment

Choose a reason for hiding this comment

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

Thank you!

@jkotas jkotas merged commit 112ef3d into dotnet:main Oct 6, 2024
157 of 163 checks passed
sirntar pushed a commit to sirntar/runtime that referenced this pull request Oct 8, 2024
@am11 am11 deleted the feature/illumos-port branch October 19, 2024 07:02

if (error != 0)
PAL_ERROR palError = InternalGetThreadDataFromHandle(pThread, hThread, &pTargetThread, &pobjThread);
if (palError != NO_ERROR)
Copy link
Member

Choose a reason for hiding this comment

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

@am11 I have just discovered that you have reversed this condition by accident. So we effectively never set the name. And if the InternalGetThreadDataFromHandle fails, the code crashes because the pTargetThread is null (I've just investigated such crash and found this to be the culprit)

Copy link
Member Author

Choose a reason for hiding this comment

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

Ops, this was an accident. Thanks for catching this. I will make a fix.

@github-actions github-actions bot locked and limited conversation to collaborators Dec 2, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-PAL-coreclr community-contribution Indicates that the PR has been added by a community member
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants