Skip to content

Unify Thread.CurrentOSThreadId between CoreCLR and NativeAOT - #135144

Merged
jkotas merged 10 commits into
dotnet:mainfrom
EgorBo:unify-current-os-thread-id
Oct 6, 2026
Merged

Unify Thread.CurrentOSThreadId between CoreCLR and NativeAOT#135144
jkotas merged 10 commits into
dotnet:mainfrom
EgorBo:unify-current-os-thread-id

Conversation

@EgorBo

@EgorBo EgorBo commented Oct 2, 2026

Copy link
Copy Markdown
Member

No description provided.

@EgorBo
Unify Thread.CurrentOSThreadId between CoreCLR and NativeAOT
13c44a3 
Implement CurrentOSThreadId in shared Thread.cs on top of existing interop
(Kernel32.GetCurrentThreadId on Windows, SystemNative_GetUInt64OSThreadId elsewhere)
and delete the CoreCLR ThreadNative_GetCurrentOSThreadId QCall and the NativeAOT
RhCurrentOSThreadId FCALL. Mark GetUInt64OSThreadId with SuppressGCTransition, and make
the WASI stub return 1 (single thread) instead of asserting.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: ac6fb33d-fc74-41c7-a804-75fbf616b155
@EgorBo
EgorBo requested a balanced review from Copilot and removed request for MichalStrehovsky October 2, 2026 23:38
@azure-pipelines

azure-pipelines Bot commented Oct 2, 2026

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 3 pipeline(s).
13 pipeline(s) were filtered out due to trigger conditions.
There may be pipelines that require an authorized user to comment /azp run to run.

@dotnet-policy-service

dotnet-policy-service Bot commented Oct 2, 2026

Copy link
Copy Markdown
Contributor

Tagging subscribers to this area: @JulieLeeMSFT, @VSadov
See info in area-owners.md if you want to be subscribed.

Copilot AI reviewed Oct 2, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot review overview

🔵 Needs a closer look

Cross-runtime interop changes need platform validation and human review, particularly for WASI and NativeAOT fail-fast consumers; builds and tests were not run.

Review effort: Balanced
Findings: None

What changed in this PR

Unifies Thread.CurrentOSThreadId across CoreCLR and NativeAOT through shared platform interop, while preserving Mono’s implementation.

Changes:

  • Routes Windows and Unix thread-ID queries through shared interop.
  • Removes redundant runtime entry points and declarations.
  • Returns thread ID 1 for single-threaded WASI.
File Description
src/​native/​libs/​System.Native/​pal_threading_wasi.c Returns the single-threaded WASI ID.
src/​libraries/​System.Private.CoreLib/​src/​System/​Threading/​Thread.cs Shares platform-specific thread-ID dispatch.
src/​libraries/​Common/​src/​Interop/​Unix/​System.Native/​Interop.Threading.cs Broadens 64-bit interop availability and suppresses GC transitions.
src/​coreclr/​vm/​qcallentrypoints.cpp Removes the obsolete QCall registration.
src/​coreclr/​vm/​comsynchronizable.h Removes the obsolete QCall declaration.
src/​coreclr/​vm/​comsynchronizable.cpp Removes the CoreCLR-specific implementation.
src/​coreclr/​System.Private.CoreLib/​src/​System/​Threading/​Thread.CoreCLR.cs Removes the managed QCall import.
src/​coreclr/​nativeaot/​System.Private.CoreLib/​src/​System/​Threading/​Thread.NativeAot.cs Removes the NativeAOT-specific property.
src/​coreclr/​nativeaot/​System.Private.CoreLib/​src/​System/​Runtime/​RuntimeImports.cs Removes the unused runtime import.
src/​coreclr/​nativeaot/​Runtime/​thread.cpp Removes the NativeAOT native entry point.

jkotas
jkotas reviewed Oct 3, 2026
View reviewed changes
Comment thread src/libraries/Common/src/Interop/Unix/System.Native/Interop.Threading.cs Outdated
jkotas
jkotas reviewed Oct 3, 2026
View reviewed changes
Comment thread src/libraries/System.Private.CoreLib/src/System/Threading/Thread.cs Outdated
@build-analysis build-analysis Bot mentioned this pull request Oct 3, 2026
Open
jkotas
jkotas reviewed Oct 3, 2026
View reviewed changes
Comment thread src/native/libs/System.Native/pal_threading_wasi.c Outdated
This was referenced Oct 3, 2026
Open
Open
jkotas
jkotas reviewed Oct 3, 2026
View reviewed changes
Comment thread src/coreclr/nativeaot/System.Private.CoreLib/src/System/Runtime/RuntimeImports.cs Outdated
@EgorBo
Address feedback: split CurrentOSThreadId per OS and remove unused he…
ce1c387 
…lpers

- Move CurrentOSThreadId into Thread.Windows.cs / Thread.Unix.cs and let Mono use it too
  (drop the Mono GetCurrentOSThreadId icall, which returned the same values).
- Rename SystemNative_GetUInt64OSThreadId to SystemNative_GetOSThreadId and delete the unused
  SystemNative_TryGetUInt32OSThreadId.
- Use minipal_get_current_thread_id in the WASI shim.
- Delete unused RhCurrentNativeThreadId.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: ac6fb33d-fc74-41c7-a804-75fbf616b155
@EgorBo

EgorBo commented Oct 3, 2026

Copy link
Copy Markdown
Member Author

@jkotas thanks for the fb, addressed

jkotas
jkotas reviewed Oct 3, 2026
View reviewed changes
Comment thread src/libraries/System.Private.CoreLib/src/System/Threading/Thread.Windows.cs Outdated
jkotas
jkotas reviewed Oct 3, 2026
View reviewed changes
Comment thread src/native/libs/System.Native/pal_threading.c Outdated
jkotas
jkotas reviewed Oct 3, 2026
View reviewed changes
Comment thread src/native/libs/System.Native/pal_threading.c Outdated
jkotas
jkotas reviewed Oct 3, 2026
View reviewed changes
Comment thread src/native/libs/System.Native/pal_threading.c Outdated
@EgorBo
Address thread ID interop review feedback
c7f33b5 
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 8acd7b8c-9618-46e1-8654-0e1b27b4480a
jkotas
jkotas reviewed Oct 4, 2026
View reviewed changes
Comment thread src/libraries/Microsoft.Win32.SystemEvents/src/Microsoft/Win32/SystemEvents.cs Outdated
jkotas
jkotas reviewed Oct 4, 2026
View reviewed changes
Comment thread src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessManager.Windows.cs Outdated
jkotas
jkotas reviewed Oct 4, 2026
View reviewed changes
Comment thread src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessManager.Windows.cs Outdated
@build-analysis build-analysis Bot mentioned this pull request Oct 4, 2026
Open
@EgorBo
Use uint for Windows thread IDs
8a8d4c5 
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 1605c6d9-7fb1-4846-bd10-06628c6bf2ea
jkotas
jkotas approved these changes Oct 4, 2026
View reviewed changes

@jkotas jkotas left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks

@jkotas
jkotas enabled auto-merge (squash) October 4, 2026 17:56
@jkotas

jkotas commented Oct 5, 2026

Copy link
Copy Markdown
Member

@lewing Could you please take a look at System.Threading.Tasks failure on Wasm? I am not able to tell whether it was somehow caused by the PR changes (the PR is changing threading, so there may be some connection).

@build-analysis build-analysis Bot mentioned this pull request Oct 6, 2026
Open
@jkotas

jkotas commented Oct 6, 2026

Copy link
Copy Markdown
Member

@EgorBo I think this may need https://github.com/dotnet/runtime/blob/main/src/coreclr/vm/wasm/generate-coreclr-helpers.cmd to fix the wasm failure.

@EgorBo
Regenerate WASM thread ID P/Invoke helpers
beaa425 
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: e652426b-8668-4590-b10e-94bbe6edf26c
@jkotas

jkotas commented Oct 6, 2026

Copy link
Copy Markdown
Member

Merge conflicts

@EgorBo

EgorBo commented Oct 6, 2026

Copy link
Copy Markdown
Member Author

Ah, oops.

EgorBo and others added 3 commits October 6, 2026 18:27
@EgorBo
Merge branch 'main' into unify-current-os-thread-id
a338a3c
@EgorBo
Update qcallentrypoints.cpp
6e96165
@EgorBo
Remove stale thread ID QCall entry after merge
f487daf 
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: ec60cb09-d94b-427d-886d-cac926a87fbb
@EgorBo
EgorBo force-pushed the unify-current-os-thread-id branch from a84cbba to f487daf Compare October 6, 2026 16:59
@jkotas jkotas mentioned this pull request Oct 6, 2026
Open
@build-analysis build-analysis Bot mentioned this pull request Oct 6, 2026
Open
@jkotas
jkotas merged commit b08c345 into dotnet:main Oct 6, 2026
165 of 168 checks passed
@dotnet-maestro dotnet-maestro Bot mentioned this pull request Oct 7, 2026
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot
Copilot review effort
Applies to this pull request for everyone.Learn more about Copilot code review.
Copilot left review comments
@jkotas jkotas jkotas approved these changes
@lateralusX lateralusX Awaiting requested review from lateralusX lateralusX is a code owner
@steveisok steveisok Awaiting requested review from steveisok steveisok is a code owner
@vitek-karas vitek-karas Awaiting requested review from vitek-karas vitek-karas is a code owner
@thaystg thaystg Awaiting requested review from thaystg thaystg is a code owner

Assignees

@EgorBo EgorBo

Labels

area-System.Threading

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@EgorBo @jkotas