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

fix thread renaming #489

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

fix thread renaming #489

wants to merge 1 commit into from

Conversation

kripergvg
Copy link

Thread renaming is quite common operation. In current implementation thread is distinguished by name and if it's renamed it's not tracked properly.
I suggest using ManagedThreadId instead. If you run the new "TestThreadRenamed" test on old implementation you can reproduce the problem.

@pdeligia
Copy link
Member

Thanks for your PR @kripergvg, however sadly AFAIK ManagedThreadId can break the runtime because although the id is unique, it can be reused by subsequent threads which makes it not a good choice for a dictionary key in this case. That is the original reason we avoided using it, and instead opted for thread naming -- although I agree with what you say its not ideal, and does not work well with cases where someone has already named threads. Our focus until recently has been higher-level C# code that uses tasks (from TPL) rather than lower-level threads, so this renaming situation was uncommon. We need to figure out a better solution moving forward to better support low-level threading scenarios.

@kripergvg
Copy link
Author

@pdeligia I see. What about using the thread instance directly as a key in dictionary?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants