Add deterministic NativeAOT regression smoke test for GC restricted reverse P/Invoke assert path - #130459

Merged
agocke merged 11 commits into
mainfrom
copilot/create-focused-regression-test-issue-110683
Aug 6, 2026
Merged

Add deterministic NativeAOT regression smoke test for GC restricted reverse P/Invoke assert path#130459
agocke merged 11 commits into
mainfrom
copilot/create-focused-regression-test-issue-110683

Conversation

CopilotAI commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

Resolves#110683.

This adds a focused NativeAOT regression test for the GC restricted callout path where background GC can invoke managed [UnmanagedCallersOnly] callbacks and hit Thread::InlineTryFastReversePInvoke assertion behavior prior to the runtime fix. The change is test-only and is intended to reliably drive the failing pre-fix path, not modify product behavior.

  • Test added

    • src/tests/nativeaot/SmokeTests/GcRestrictedCalloutReversePInvoke/
    • New smoke test project plus program harness.
  • Repro wiring

    • Registers tracker support via ComWrappers.RegisterForTrackerSupport(...).
    • Creates a tracker object through MockReferenceTrackerRuntime so GC restricted tracker callbacks are active (GCStopCollection path).
  • Deterministic triggering

    • Forces repeated concurrent Gen2 collections (GC.Collect(..., blocking:false)).
    • Runs bounded allocation pressure on a background thread.
    • Uses timeout + synchronization (SpinWait + elapsed bound) instead of unbounded probabilistic loops.
  • Scope/guards

    • Configured as process-isolated NativeAOT smoke test.
    • Excludes Apple mobile and Android targets where the required native dependency is not available in this leg.
    • Keeps coverage broad otherwise (not win-x86-only).
  • Expectation documented

    • Source comment references dotnet/runtime#110683 and states expected behavior:
      • release: test should complete,
      • assertion-enabled runtime (before product fix): can fail on the target assertion path.
  • Environment limitation observed while validating command

    • In this environment, Release NativeAOT targeted test build failed due to missing Microsoft.DotNet.ILCompiler.SingleEntry.targets; Checked configuration was used for targeted execution.
GC.Collect(2,GCCollectionMode.Forced,blocking:false,compacting:false);if(GC.GetGCMemoryInfo().Generation==2&&GC.GetGCMemoryInfo().Concurrent){// Concurrent Gen2 observed while tracker GC callbacks are registered.// This is the targeted repro envelope for dotnet/runtime#110683.}

Co-authored-by: MichalStrehovsky <13110571+MichalStrehovsky@users.noreply.github.com>
CopilotAI requested review from Copilot and removed request for CopilotJuly 10, 2026 03:33
CopilotAI changed the title [WIP] Add regression test for NativeAOT runtime assertion failureAdd deterministic NativeAOT regression smoke test for GC restricted reverse P/Invoke assert pathJul 10, 2026
@dotnet-policy-service

Copy link
Copy Markdown
Contributor

Tagging subscribers to this area: @agocke, @dotnet/ilc-contrib
See info in area-owners.md if you want to be subscribed.

Co-authored-by: MichalStrehovsky <13110571+MichalStrehovsky@users.noreply.github.com>
@MichalStrehovsky

Copy link
Copy Markdown
Member

@copilot now fix the assert failure by weakening the assertion as described in #110683.

Co-authored-by: MichalStrehovsky <13110571+MichalStrehovsky@users.noreply.github.com>
CopilotAI requested review from Copilot and removed request for CopilotJuly 14, 2026 07:40

CopilotAI commented Jul 14, 2026

Copy link
Copy Markdown
ContributorAuthor

@copilot now fix the assert failure by weakening the assertion as described in #110683.

Fixed in 4413efc — the NativeAOT reverse P/Invoke fast path now asserts IsGCSpecial() for DoNotTriggerGc threads instead of requiring ThreadStore::IsTrapThreadsRequested().

CopilotAI 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.

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

CopilotAI review requested due to automatic review settings July 17, 2026 06:14

CopilotAI 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.

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

CopilotAI review requested due to automatic review settings July 21, 2026 08:15

CopilotAI 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.

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

Comments suppressed due to low confidence (1)

src/tests/Interop/COM/ComWrappers/GcRestrictedCalloutReversePInvoke/GcRestrictedCalloutReversePInvoke.csproj:21

  • This project uses xUnit ([Fact] in Program.cs) but unlike the other ComWrappers interop test projects in this directory it doesn’t reference $(TestLibraryProjectPath). As-is, this will likely fail to compile due to missing xUnit/test infrastructure references.
 <ItemGroup>
<Compile Include="../Common.cs" />
<Compile Include="Program.cs" />
</ItemGroup>
<ItemGroup>
<CMakeProjectReference Include="../MockReferenceTrackerRuntime/CMakeLists.txt" />
</ItemGroup>
</Project>

CopilotAI review requested due to automatic review settings July 28, 2026 12:39
@MichalStrehovsky
MichalStrehovsky marked this pull request as ready for review July 28, 2026 12:40
@azure-pipelines

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.

@MichalStrehovsky

Copy link
Copy Markdown
Member

We now have a test that hits the assert without this fix and works with the fix. Is the test valuable enough to keep?

Cc @VSadov

CopilotAI 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.

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (4)

src/tests/Interop/COM/ComWrappers/GcRestrictedCalloutReversePInvoke/GcRestrictedCalloutReversePInvoke.csproj:16

  • This test project uses xUnit ([Fact] in Program.cs) but the csproj is missing the standard that all other ComWrappers test projects in this directory use. Without it, the project may not compile or may miss test infrastructure references.
 <ItemGroup>
<Compile Include="../Common.cs" />
<Compile Include="Program.cs" />
</ItemGroup>

src/tests/Interop/COM/ComWrappers/GcRestrictedCalloutReversePInvoke/GcRestrictedCalloutReversePInvoke.csproj:5

  • This project is described as a NativeAOT smoke test in the PR metadata, but it’s currently added under src/tests/Interop/COM/ComWrappers (not src/tests/nativeaot/SmokeTests). If it’s intended to run in the NativeAOT smoke test legs, it likely needs to live under the nativeaot test tree (or have additional wiring) to be discovered/executed there.
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<CLRTestPriority>0</CLRTestPriority>
<AllowUnsafeBlocks>true</AllowUnsafeBlocks>
<!-- Test interacts with GC collections, also needed for CLRTestTargetUnsupported below -->

src/tests/Interop/COM/ComWrappers/GcRestrictedCalloutReversePInvoke/Program.cs:63

  • Use xUnit assertions for test failures instead of throwing Exception. This keeps failures consistent with the rest of the ComWrappers tests and produces cleaner output in test reports.
 if (!observedConcurrentGen2)
{
throw new Exception($"Timed out after {timeout} waiting for a concurrent Gen2 GC. Initial count: {initialGen2Collections}, final count: {GC.CollectionCount(2)}.");
}

src/coreclr/nativeaot/Runtime/thread.inl:208

  • The PR description states the change is test-only and does not modify product behavior, but this hunk changes a runtime assertion condition in Thread::InlineTryFastReversePInvoke. Please update the PR description/scope accordingly (or split the runtime change into the appropriate fix PR if this PR is meant to be test-only).
 if (IsDoNotTriggerGcSet())
{
// We expect this scenario only when EE is stopped or we're on a GC worker thread.
ASSERT(ThreadStore::IsTrapThreadsRequested() || IsGCSpecial());
// no need to do anything

@agocke
agocke merged commit 55fba98 into mainAug 6, 2026
117 checks passed
@agocke
agocke deleted the copilot/create-focused-regression-test-issue-110683 branch August 6, 2026 18:18
@dotnet-milestone-botdotnet-milestone-botBot added this to the 11.0-rc1 milestone Aug 7, 2026
jkoritzinsky added a commit that referenced this pull request Aug 12, 2026
…require multithreading support (#132176)
`GcRestrictedCalloutReversePInvoke` fails under stress runs because
`ComWrappers.RegisterForTrackerSupport` throws
`PlatformNotSupportedException` under AnyGCStress/AnyJitStress
configurations (introduced by #130459), and the test inherently depends
on background GC/multithreading.
## Changes
- Added `[ActiveIssue]` attributes disabling the test under
`AnyGCStress` and `AnyJitStress` coreclr configurations, via
`CoreClrConfigurationDetection.IsGCStress` and
`CoreClrConfigurationDetection.IsAnyJitStress`.
- Replaced `[Fact]` with `[ConditionalFact(typeof(PlatformDetection),
nameof(PlatformDetection.IsMultithreadingSupported))]` so the test only
runs where multithreading is supported.
<!-- START COPILOT CODING AGENT SUFFIX -->
- Fixes#131989
---------
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: jkoritzinsky <1571408+jkoritzinsky@users.noreply.github.com>
Co-authored-by: Jeremy Koritzinsky <jekoritz@microsoft.com>
Co-authored-by: Jan Kotas <jkotas@microsoft.com>
MichalStrehovsky added a commit that referenced this pull request Aug 31, 2026
Resolves#132939.
I wasn't sure if we want the test in the first place
(#130459 (comment))
and now that it's known to be flaky, the decision is obvious.
---------
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: MichalStrehovsky <13110571+MichalStrehovsky@users.noreply.github.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ASSERT(ThreadStore::IsTrapThreadsRequested()) in Thread::InlineTryFastReversePInvoke

4 participants

@MichalStrehovsky@agocke
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
Skip to content

Add deterministic NativeAOT regression smoke test for GC restricted reverse P/Invoke assert path - #130459

Merged
agocke merged 11 commits into
mainfrom
copilot/create-focused-regression-test-issue-110683
Aug 6, 2026
Merged

Add deterministic NativeAOT regression smoke test for GC restricted reverse P/Invoke assert path#130459
agocke merged 11 commits into
mainfrom
copilot/create-focused-regression-test-issue-110683

Conversation

CopilotAI commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

Resolves#110683.

This adds a focused NativeAOT regression test for the GC restricted callout path where background GC can invoke managed [UnmanagedCallersOnly] callbacks and hit Thread::InlineTryFastReversePInvoke assertion behavior prior to the runtime fix. The change is test-only and is intended to reliably drive the failing pre-fix path, not modify product behavior.

  • Test added

    • src/tests/nativeaot/SmokeTests/GcRestrictedCalloutReversePInvoke/
    • New smoke test project plus program harness.
  • Repro wiring

    • Registers tracker support via ComWrappers.RegisterForTrackerSupport(...).
    • Creates a tracker object through MockReferenceTrackerRuntime so GC restricted tracker callbacks are active (GCStopCollection path).
  • Deterministic triggering

    • Forces repeated concurrent Gen2 collections (GC.Collect(..., blocking:false)).
    • Runs bounded allocation pressure on a background thread.
    • Uses timeout + synchronization (SpinWait + elapsed bound) instead of unbounded probabilistic loops.
  • Scope/guards

    • Configured as process-isolated NativeAOT smoke test.
    • Excludes Apple mobile and Android targets where the required native dependency is not available in this leg.
    • Keeps coverage broad otherwise (not win-x86-only).
  • Expectation documented

    • Source comment references dotnet/runtime#110683 and states expected behavior:
      • release: test should complete,
      • assertion-enabled runtime (before product fix): can fail on the target assertion path.
  • Environment limitation observed while validating command

    • In this environment, Release NativeAOT targeted test build failed due to missing Microsoft.DotNet.ILCompiler.SingleEntry.targets; Checked configuration was used for targeted execution.
GC.Collect(2,GCCollectionMode.Forced,blocking:false,compacting:false);if(GC.GetGCMemoryInfo().Generation==2&&GC.GetGCMemoryInfo().Concurrent){// Concurrent Gen2 observed while tracker GC callbacks are registered.// This is the targeted repro envelope for dotnet/runtime#110683.}

Co-authored-by: MichalStrehovsky <13110571+MichalStrehovsky@users.noreply.github.com>
CopilotAI requested review from Copilot and removed request for CopilotJuly 10, 2026 03:33
CopilotAI changed the title [WIP] Add regression test for NativeAOT runtime assertion failureAdd deterministic NativeAOT regression smoke test for GC restricted reverse P/Invoke assert pathJul 10, 2026
@dotnet-policy-service

Copy link
Copy Markdown
Contributor

Tagging subscribers to this area: @agocke, @dotnet/ilc-contrib
See info in area-owners.md if you want to be subscribed.

Co-authored-by: MichalStrehovsky <13110571+MichalStrehovsky@users.noreply.github.com>
@MichalStrehovsky

Copy link
Copy Markdown
Member

@copilot now fix the assert failure by weakening the assertion as described in #110683.

Co-authored-by: MichalStrehovsky <13110571+MichalStrehovsky@users.noreply.github.com>
CopilotAI requested review from Copilot and removed request for CopilotJuly 14, 2026 07:40

CopilotAI commented Jul 14, 2026

Copy link
Copy Markdown
ContributorAuthor

@copilot now fix the assert failure by weakening the assertion as described in #110683.

Fixed in 4413efc — the NativeAOT reverse P/Invoke fast path now asserts IsGCSpecial() for DoNotTriggerGc threads instead of requiring ThreadStore::IsTrapThreadsRequested().

CopilotAI 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.

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

CopilotAI review requested due to automatic review settings July 17, 2026 06:14

CopilotAI 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.

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

CopilotAI review requested due to automatic review settings July 21, 2026 08:15

CopilotAI 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.

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

Comments suppressed due to low confidence (1)

src/tests/Interop/COM/ComWrappers/GcRestrictedCalloutReversePInvoke/GcRestrictedCalloutReversePInvoke.csproj:21

  • This project uses xUnit ([Fact] in Program.cs) but unlike the other ComWrappers interop test projects in this directory it doesn’t reference $(TestLibraryProjectPath). As-is, this will likely fail to compile due to missing xUnit/test infrastructure references.
 <ItemGroup>
<Compile Include="../Common.cs" />
<Compile Include="Program.cs" />
</ItemGroup>
<ItemGroup>
<CMakeProjectReference Include="../MockReferenceTrackerRuntime/CMakeLists.txt" />
</ItemGroup>
</Project>

CopilotAI review requested due to automatic review settings July 28, 2026 12:39
@MichalStrehovsky
MichalStrehovsky marked this pull request as ready for review July 28, 2026 12:40
@azure-pipelines

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.

@MichalStrehovsky

Copy link
Copy Markdown
Member

We now have a test that hits the assert without this fix and works with the fix. Is the test valuable enough to keep?

Cc @VSadov

CopilotAI 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.

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (4)

src/tests/Interop/COM/ComWrappers/GcRestrictedCalloutReversePInvoke/GcRestrictedCalloutReversePInvoke.csproj:16

  • This test project uses xUnit ([Fact] in Program.cs) but the csproj is missing the standard that all other ComWrappers test projects in this directory use. Without it, the project may not compile or may miss test infrastructure references.
 <ItemGroup>
<Compile Include="../Common.cs" />
<Compile Include="Program.cs" />
</ItemGroup>

src/tests/Interop/COM/ComWrappers/GcRestrictedCalloutReversePInvoke/GcRestrictedCalloutReversePInvoke.csproj:5

  • This project is described as a NativeAOT smoke test in the PR metadata, but it’s currently added under src/tests/Interop/COM/ComWrappers (not src/tests/nativeaot/SmokeTests). If it’s intended to run in the NativeAOT smoke test legs, it likely needs to live under the nativeaot test tree (or have additional wiring) to be discovered/executed there.
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<CLRTestPriority>0</CLRTestPriority>
<AllowUnsafeBlocks>true</AllowUnsafeBlocks>
<!-- Test interacts with GC collections, also needed for CLRTestTargetUnsupported below -->

src/tests/Interop/COM/ComWrappers/GcRestrictedCalloutReversePInvoke/Program.cs:63

  • Use xUnit assertions for test failures instead of throwing Exception. This keeps failures consistent with the rest of the ComWrappers tests and produces cleaner output in test reports.
 if (!observedConcurrentGen2)
{
throw new Exception($"Timed out after {timeout} waiting for a concurrent Gen2 GC. Initial count: {initialGen2Collections}, final count: {GC.CollectionCount(2)}.");
}

src/coreclr/nativeaot/Runtime/thread.inl:208

  • The PR description states the change is test-only and does not modify product behavior, but this hunk changes a runtime assertion condition in Thread::InlineTryFastReversePInvoke. Please update the PR description/scope accordingly (or split the runtime change into the appropriate fix PR if this PR is meant to be test-only).
 if (IsDoNotTriggerGcSet())
{
// We expect this scenario only when EE is stopped or we're on a GC worker thread.
ASSERT(ThreadStore::IsTrapThreadsRequested() || IsGCSpecial());
// no need to do anything

@agocke
agocke merged commit 55fba98 into mainAug 6, 2026
117 checks passed
@agocke
agocke deleted the copilot/create-focused-regression-test-issue-110683 branch August 6, 2026 18:18
@dotnet-milestone-botdotnet-milestone-botBot added this to the 11.0-rc1 milestone Aug 7, 2026
jkoritzinsky added a commit that referenced this pull request Aug 12, 2026
…require multithreading support (#132176)
`GcRestrictedCalloutReversePInvoke` fails under stress runs because
`ComWrappers.RegisterForTrackerSupport` throws
`PlatformNotSupportedException` under AnyGCStress/AnyJitStress
configurations (introduced by #130459), and the test inherently depends
on background GC/multithreading.
## Changes
- Added `[ActiveIssue]` attributes disabling the test under
`AnyGCStress` and `AnyJitStress` coreclr configurations, via
`CoreClrConfigurationDetection.IsGCStress` and
`CoreClrConfigurationDetection.IsAnyJitStress`.
- Replaced `[Fact]` with `[ConditionalFact(typeof(PlatformDetection),
nameof(PlatformDetection.IsMultithreadingSupported))]` so the test only
runs where multithreading is supported.
<!-- START COPILOT CODING AGENT SUFFIX -->
- Fixes#131989
---------
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: jkoritzinsky <1571408+jkoritzinsky@users.noreply.github.com>
Co-authored-by: Jeremy Koritzinsky <jekoritz@microsoft.com>
Co-authored-by: Jan Kotas <jkotas@microsoft.com>
MichalStrehovsky added a commit that referenced this pull request Aug 31, 2026
Resolves#132939.
I wasn't sure if we want the test in the first place
(#130459 (comment))
and now that it's known to be flaky, the decision is obvious.
---------
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: MichalStrehovsky <13110571+MichalStrehovsky@users.noreply.github.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ASSERT(ThreadStore::IsTrapThreadsRequested()) in Thread::InlineTryFastReversePInvoke

4 participants

@MichalStrehovsky@agocke
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix"); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Add deterministic NativeAOT regression smoke test for GC restricted reverse P/Invoke assert path - #130459

Merged
agocke merged 11 commits into
mainfrom
copilot/create-focused-regression-test-issue-110683
Aug 6, 2026
Merged

Add deterministic NativeAOT regression smoke test for GC restricted reverse P/Invoke assert path#130459
agocke merged 11 commits into
mainfrom
copilot/create-focused-regression-test-issue-110683

Conversation

CopilotAI commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

Resolves#110683.

This adds a focused NativeAOT regression test for the GC restricted callout path where background GC can invoke managed [UnmanagedCallersOnly] callbacks and hit Thread::InlineTryFastReversePInvoke assertion behavior prior to the runtime fix. The change is test-only and is intended to reliably drive the failing pre-fix path, not modify product behavior.

  • Test added

    • src/tests/nativeaot/SmokeTests/GcRestrictedCalloutReversePInvoke/
    • New smoke test project plus program harness.
  • Repro wiring

    • Registers tracker support via ComWrappers.RegisterForTrackerSupport(...).
    • Creates a tracker object through MockReferenceTrackerRuntime so GC restricted tracker callbacks are active (GCStopCollection path).
  • Deterministic triggering

    • Forces repeated concurrent Gen2 collections (GC.Collect(..., blocking:false)).
    • Runs bounded allocation pressure on a background thread.
    • Uses timeout + synchronization (SpinWait + elapsed bound) instead of unbounded probabilistic loops.
  • Scope/guards

    • Configured as process-isolated NativeAOT smoke test.
    • Excludes Apple mobile and Android targets where the required native dependency is not available in this leg.
    • Keeps coverage broad otherwise (not win-x86-only).
  • Expectation documented

    • Source comment references dotnet/runtime#110683 and states expected behavior:
      • release: test should complete,
      • assertion-enabled runtime (before product fix): can fail on the target assertion path.
  • Environment limitation observed while validating command

    • In this environment, Release NativeAOT targeted test build failed due to missing Microsoft.DotNet.ILCompiler.SingleEntry.targets; Checked configuration was used for targeted execution.
GC.Collect(2,GCCollectionMode.Forced,blocking:false,compacting:false);if(GC.GetGCMemoryInfo().Generation==2&&GC.GetGCMemoryInfo().Concurrent){// Concurrent Gen2 observed while tracker GC callbacks are registered.// This is the targeted repro envelope for dotnet/runtime#110683.}

Co-authored-by: MichalStrehovsky <13110571+MichalStrehovsky@users.noreply.github.com>
CopilotAI requested review from Copilot and removed request for CopilotJuly 10, 2026 03:33
CopilotAI changed the title [WIP] Add regression test for NativeAOT runtime assertion failureAdd deterministic NativeAOT regression smoke test for GC restricted reverse P/Invoke assert pathJul 10, 2026
@dotnet-policy-service

Copy link
Copy Markdown
Contributor

Tagging subscribers to this area: @agocke, @dotnet/ilc-contrib
See info in area-owners.md if you want to be subscribed.

Co-authored-by: MichalStrehovsky <13110571+MichalStrehovsky@users.noreply.github.com>
@MichalStrehovsky

Copy link
Copy Markdown
Member

@copilot now fix the assert failure by weakening the assertion as described in #110683.

Co-authored-by: MichalStrehovsky <13110571+MichalStrehovsky@users.noreply.github.com>
CopilotAI requested review from Copilot and removed request for CopilotJuly 14, 2026 07:40

CopilotAI commented Jul 14, 2026

Copy link
Copy Markdown
ContributorAuthor

@copilot now fix the assert failure by weakening the assertion as described in #110683.

Fixed in 4413efc — the NativeAOT reverse P/Invoke fast path now asserts IsGCSpecial() for DoNotTriggerGc threads instead of requiring ThreadStore::IsTrapThreadsRequested().

CopilotAI 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.

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

CopilotAI review requested due to automatic review settings July 17, 2026 06:14

CopilotAI 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.

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

CopilotAI review requested due to automatic review settings July 21, 2026 08:15

CopilotAI 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.

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

Comments suppressed due to low confidence (1)

src/tests/Interop/COM/ComWrappers/GcRestrictedCalloutReversePInvoke/GcRestrictedCalloutReversePInvoke.csproj:21

  • This project uses xUnit ([Fact] in Program.cs) but unlike the other ComWrappers interop test projects in this directory it doesn’t reference $(TestLibraryProjectPath). As-is, this will likely fail to compile due to missing xUnit/test infrastructure references.
 <ItemGroup>
<Compile Include="../Common.cs" />
<Compile Include="Program.cs" />
</ItemGroup>
<ItemGroup>
<CMakeProjectReference Include="../MockReferenceTrackerRuntime/CMakeLists.txt" />
</ItemGroup>
</Project>

CopilotAI review requested due to automatic review settings July 28, 2026 12:39
@MichalStrehovsky
MichalStrehovsky marked this pull request as ready for review July 28, 2026 12:40
@azure-pipelines

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.

@MichalStrehovsky

Copy link
Copy Markdown
Member

We now have a test that hits the assert without this fix and works with the fix. Is the test valuable enough to keep?

Cc @VSadov

CopilotAI 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.

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (4)

src/tests/Interop/COM/ComWrappers/GcRestrictedCalloutReversePInvoke/GcRestrictedCalloutReversePInvoke.csproj:16

  • This test project uses xUnit ([Fact] in Program.cs) but the csproj is missing the standard that all other ComWrappers test projects in this directory use. Without it, the project may not compile or may miss test infrastructure references.
 <ItemGroup>
<Compile Include="../Common.cs" />
<Compile Include="Program.cs" />
</ItemGroup>

src/tests/Interop/COM/ComWrappers/GcRestrictedCalloutReversePInvoke/GcRestrictedCalloutReversePInvoke.csproj:5

  • This project is described as a NativeAOT smoke test in the PR metadata, but it’s currently added under src/tests/Interop/COM/ComWrappers (not src/tests/nativeaot/SmokeTests). If it’s intended to run in the NativeAOT smoke test legs, it likely needs to live under the nativeaot test tree (or have additional wiring) to be discovered/executed there.
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<CLRTestPriority>0</CLRTestPriority>
<AllowUnsafeBlocks>true</AllowUnsafeBlocks>
<!-- Test interacts with GC collections, also needed for CLRTestTargetUnsupported below -->

src/tests/Interop/COM/ComWrappers/GcRestrictedCalloutReversePInvoke/Program.cs:63

  • Use xUnit assertions for test failures instead of throwing Exception. This keeps failures consistent with the rest of the ComWrappers tests and produces cleaner output in test reports.
 if (!observedConcurrentGen2)
{
throw new Exception($"Timed out after {timeout} waiting for a concurrent Gen2 GC. Initial count: {initialGen2Collections}, final count: {GC.CollectionCount(2)}.");
}

src/coreclr/nativeaot/Runtime/thread.inl:208

  • The PR description states the change is test-only and does not modify product behavior, but this hunk changes a runtime assertion condition in Thread::InlineTryFastReversePInvoke. Please update the PR description/scope accordingly (or split the runtime change into the appropriate fix PR if this PR is meant to be test-only).
 if (IsDoNotTriggerGcSet())
{
// We expect this scenario only when EE is stopped or we're on a GC worker thread.
ASSERT(ThreadStore::IsTrapThreadsRequested() || IsGCSpecial());
// no need to do anything

@agocke
agocke merged commit 55fba98 into mainAug 6, 2026
117 checks passed
@agocke
agocke deleted the copilot/create-focused-regression-test-issue-110683 branch August 6, 2026 18:18
@dotnet-milestone-botdotnet-milestone-botBot added this to the 11.0-rc1 milestone Aug 7, 2026
jkoritzinsky added a commit that referenced this pull request Aug 12, 2026
…require multithreading support (#132176)
`GcRestrictedCalloutReversePInvoke` fails under stress runs because
`ComWrappers.RegisterForTrackerSupport` throws
`PlatformNotSupportedException` under AnyGCStress/AnyJitStress
configurations (introduced by #130459), and the test inherently depends
on background GC/multithreading.
## Changes
- Added `[ActiveIssue]` attributes disabling the test under
`AnyGCStress` and `AnyJitStress` coreclr configurations, via
`CoreClrConfigurationDetection.IsGCStress` and
`CoreClrConfigurationDetection.IsAnyJitStress`.
- Replaced `[Fact]` with `[ConditionalFact(typeof(PlatformDetection),
nameof(PlatformDetection.IsMultithreadingSupported))]` so the test only
runs where multithreading is supported.
<!-- START COPILOT CODING AGENT SUFFIX -->
- Fixes#131989
---------
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: jkoritzinsky <1571408+jkoritzinsky@users.noreply.github.com>
Co-authored-by: Jeremy Koritzinsky <jekoritz@microsoft.com>
Co-authored-by: Jan Kotas <jkotas@microsoft.com>
MichalStrehovsky added a commit that referenced this pull request Aug 31, 2026
Resolves#132939.
I wasn't sure if we want the test in the first place
(#130459 (comment))
and now that it's known to be flaky, the decision is obvious.
---------
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: MichalStrehovsky <13110571+MichalStrehovsky@users.noreply.github.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ASSERT(ThreadStore::IsTrapThreadsRequested()) in Thread::InlineTryFastReversePInvoke

4 participants

@MichalStrehovsky@agocke
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms"); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Add deterministic NativeAOT regression smoke test for GC restricted reverse P/Invoke assert path - #130459

Merged
agocke merged 11 commits into
mainfrom
copilot/create-focused-regression-test-issue-110683
Aug 6, 2026
Merged

Add deterministic NativeAOT regression smoke test for GC restricted reverse P/Invoke assert path#130459
agocke merged 11 commits into
mainfrom
copilot/create-focused-regression-test-issue-110683

Conversation

CopilotAI commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

Resolves#110683.

This adds a focused NativeAOT regression test for the GC restricted callout path where background GC can invoke managed [UnmanagedCallersOnly] callbacks and hit Thread::InlineTryFastReversePInvoke assertion behavior prior to the runtime fix. The change is test-only and is intended to reliably drive the failing pre-fix path, not modify product behavior.

  • Test added

    • src/tests/nativeaot/SmokeTests/GcRestrictedCalloutReversePInvoke/
    • New smoke test project plus program harness.
  • Repro wiring

    • Registers tracker support via ComWrappers.RegisterForTrackerSupport(...).
    • Creates a tracker object through MockReferenceTrackerRuntime so GC restricted tracker callbacks are active (GCStopCollection path).
  • Deterministic triggering

    • Forces repeated concurrent Gen2 collections (GC.Collect(..., blocking:false)).
    • Runs bounded allocation pressure on a background thread.
    • Uses timeout + synchronization (SpinWait + elapsed bound) instead of unbounded probabilistic loops.
  • Scope/guards

    • Configured as process-isolated NativeAOT smoke test.
    • Excludes Apple mobile and Android targets where the required native dependency is not available in this leg.
    • Keeps coverage broad otherwise (not win-x86-only).
  • Expectation documented

    • Source comment references dotnet/runtime#110683 and states expected behavior:
      • release: test should complete,
      • assertion-enabled runtime (before product fix): can fail on the target assertion path.
  • Environment limitation observed while validating command

    • In this environment, Release NativeAOT targeted test build failed due to missing Microsoft.DotNet.ILCompiler.SingleEntry.targets; Checked configuration was used for targeted execution.
GC.Collect(2,GCCollectionMode.Forced,blocking:false,compacting:false);if(GC.GetGCMemoryInfo().Generation==2&&GC.GetGCMemoryInfo().Concurrent){// Concurrent Gen2 observed while tracker GC callbacks are registered.// This is the targeted repro envelope for dotnet/runtime#110683.}

Co-authored-by: MichalStrehovsky <13110571+MichalStrehovsky@users.noreply.github.com>
CopilotAI requested review from Copilot and removed request for CopilotJuly 10, 2026 03:33
CopilotAI changed the title [WIP] Add regression test for NativeAOT runtime assertion failureAdd deterministic NativeAOT regression smoke test for GC restricted reverse P/Invoke assert pathJul 10, 2026
@dotnet-policy-service

Copy link
Copy Markdown
Contributor

Tagging subscribers to this area: @agocke, @dotnet/ilc-contrib
See info in area-owners.md if you want to be subscribed.

Co-authored-by: MichalStrehovsky <13110571+MichalStrehovsky@users.noreply.github.com>
@MichalStrehovsky

Copy link
Copy Markdown
Member

@copilot now fix the assert failure by weakening the assertion as described in #110683.

Co-authored-by: MichalStrehovsky <13110571+MichalStrehovsky@users.noreply.github.com>
CopilotAI requested review from Copilot and removed request for CopilotJuly 14, 2026 07:40

CopilotAI commented Jul 14, 2026

Copy link
Copy Markdown
ContributorAuthor

@copilot now fix the assert failure by weakening the assertion as described in #110683.

Fixed in 4413efc — the NativeAOT reverse P/Invoke fast path now asserts IsGCSpecial() for DoNotTriggerGc threads instead of requiring ThreadStore::IsTrapThreadsRequested().

CopilotAI 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.

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

CopilotAI review requested due to automatic review settings July 17, 2026 06:14

CopilotAI 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.

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

CopilotAI review requested due to automatic review settings July 21, 2026 08:15

CopilotAI 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.

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

Comments suppressed due to low confidence (1)

src/tests/Interop/COM/ComWrappers/GcRestrictedCalloutReversePInvoke/GcRestrictedCalloutReversePInvoke.csproj:21

  • This project uses xUnit ([Fact] in Program.cs) but unlike the other ComWrappers interop test projects in this directory it doesn’t reference $(TestLibraryProjectPath). As-is, this will likely fail to compile due to missing xUnit/test infrastructure references.
 <ItemGroup>
<Compile Include="../Common.cs" />
<Compile Include="Program.cs" />
</ItemGroup>
<ItemGroup>
<CMakeProjectReference Include="../MockReferenceTrackerRuntime/CMakeLists.txt" />
</ItemGroup>
</Project>

CopilotAI review requested due to automatic review settings July 28, 2026 12:39
@MichalStrehovsky
MichalStrehovsky marked this pull request as ready for review July 28, 2026 12:40
@azure-pipelines

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.

@MichalStrehovsky

Copy link
Copy Markdown
Member

We now have a test that hits the assert without this fix and works with the fix. Is the test valuable enough to keep?

Cc @VSadov

CopilotAI 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.

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (4)

src/tests/Interop/COM/ComWrappers/GcRestrictedCalloutReversePInvoke/GcRestrictedCalloutReversePInvoke.csproj:16

  • This test project uses xUnit ([Fact] in Program.cs) but the csproj is missing the standard that all other ComWrappers test projects in this directory use. Without it, the project may not compile or may miss test infrastructure references.
 <ItemGroup>
<Compile Include="../Common.cs" />
<Compile Include="Program.cs" />
</ItemGroup>

src/tests/Interop/COM/ComWrappers/GcRestrictedCalloutReversePInvoke/GcRestrictedCalloutReversePInvoke.csproj:5

  • This project is described as a NativeAOT smoke test in the PR metadata, but it’s currently added under src/tests/Interop/COM/ComWrappers (not src/tests/nativeaot/SmokeTests). If it’s intended to run in the NativeAOT smoke test legs, it likely needs to live under the nativeaot test tree (or have additional wiring) to be discovered/executed there.
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<CLRTestPriority>0</CLRTestPriority>
<AllowUnsafeBlocks>true</AllowUnsafeBlocks>
<!-- Test interacts with GC collections, also needed for CLRTestTargetUnsupported below -->

src/tests/Interop/COM/ComWrappers/GcRestrictedCalloutReversePInvoke/Program.cs:63

  • Use xUnit assertions for test failures instead of throwing Exception. This keeps failures consistent with the rest of the ComWrappers tests and produces cleaner output in test reports.
 if (!observedConcurrentGen2)
{
throw new Exception($"Timed out after {timeout} waiting for a concurrent Gen2 GC. Initial count: {initialGen2Collections}, final count: {GC.CollectionCount(2)}.");
}

src/coreclr/nativeaot/Runtime/thread.inl:208

  • The PR description states the change is test-only and does not modify product behavior, but this hunk changes a runtime assertion condition in Thread::InlineTryFastReversePInvoke. Please update the PR description/scope accordingly (or split the runtime change into the appropriate fix PR if this PR is meant to be test-only).
 if (IsDoNotTriggerGcSet())
{
// We expect this scenario only when EE is stopped or we're on a GC worker thread.
ASSERT(ThreadStore::IsTrapThreadsRequested() || IsGCSpecial());
// no need to do anything

@agocke
agocke merged commit 55fba98 into mainAug 6, 2026
117 checks passed
@agocke
agocke deleted the copilot/create-focused-regression-test-issue-110683 branch August 6, 2026 18:18
@dotnet-milestone-botdotnet-milestone-botBot added this to the 11.0-rc1 milestone Aug 7, 2026
jkoritzinsky added a commit that referenced this pull request Aug 12, 2026
…require multithreading support (#132176)
`GcRestrictedCalloutReversePInvoke` fails under stress runs because
`ComWrappers.RegisterForTrackerSupport` throws
`PlatformNotSupportedException` under AnyGCStress/AnyJitStress
configurations (introduced by #130459), and the test inherently depends
on background GC/multithreading.
## Changes
- Added `[ActiveIssue]` attributes disabling the test under
`AnyGCStress` and `AnyJitStress` coreclr configurations, via
`CoreClrConfigurationDetection.IsGCStress` and
`CoreClrConfigurationDetection.IsAnyJitStress`.
- Replaced `[Fact]` with `[ConditionalFact(typeof(PlatformDetection),
nameof(PlatformDetection.IsMultithreadingSupported))]` so the test only
runs where multithreading is supported.
<!-- START COPILOT CODING AGENT SUFFIX -->
- Fixes#131989
---------
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: jkoritzinsky <1571408+jkoritzinsky@users.noreply.github.com>
Co-authored-by: Jeremy Koritzinsky <jekoritz@microsoft.com>
Co-authored-by: Jan Kotas <jkotas@microsoft.com>
MichalStrehovsky added a commit that referenced this pull request Aug 31, 2026
Resolves#132939.
I wasn't sure if we want the test in the first place
(#130459 (comment))
and now that it's known to be flaky, the decision is obvious.
---------
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: MichalStrehovsky <13110571+MichalStrehovsky@users.noreply.github.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ASSERT(ThreadStore::IsTrapThreadsRequested()) in Thread::InlineTryFastReversePInvoke

4 participants

@MichalStrehovsky@agocke
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links"); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + '
Skip to content

Add deterministic NativeAOT regression smoke test for GC restricted reverse P/Invoke assert path - #130459

Merged
agocke merged 11 commits into
mainfrom
copilot/create-focused-regression-test-issue-110683
Aug 6, 2026
Merged

Add deterministic NativeAOT regression smoke test for GC restricted reverse P/Invoke assert path#130459
agocke merged 11 commits into
mainfrom
copilot/create-focused-regression-test-issue-110683

Conversation

CopilotAI commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

Resolves#110683.

This adds a focused NativeAOT regression test for the GC restricted callout path where background GC can invoke managed [UnmanagedCallersOnly] callbacks and hit Thread::InlineTryFastReversePInvoke assertion behavior prior to the runtime fix. The change is test-only and is intended to reliably drive the failing pre-fix path, not modify product behavior.

  • Test added

    • src/tests/nativeaot/SmokeTests/GcRestrictedCalloutReversePInvoke/
    • New smoke test project plus program harness.
  • Repro wiring

    • Registers tracker support via ComWrappers.RegisterForTrackerSupport(...).
    • Creates a tracker object through MockReferenceTrackerRuntime so GC restricted tracker callbacks are active (GCStopCollection path).
  • Deterministic triggering

    • Forces repeated concurrent Gen2 collections (GC.Collect(..., blocking:false)).
    • Runs bounded allocation pressure on a background thread.
    • Uses timeout + synchronization (SpinWait + elapsed bound) instead of unbounded probabilistic loops.
  • Scope/guards

    • Configured as process-isolated NativeAOT smoke test.
    • Excludes Apple mobile and Android targets where the required native dependency is not available in this leg.
    • Keeps coverage broad otherwise (not win-x86-only).
  • Expectation documented

    • Source comment references dotnet/runtime#110683 and states expected behavior:
      • release: test should complete,
      • assertion-enabled runtime (before product fix): can fail on the target assertion path.
  • Environment limitation observed while validating command

    • In this environment, Release NativeAOT targeted test build failed due to missing Microsoft.DotNet.ILCompiler.SingleEntry.targets; Checked configuration was used for targeted execution.
GC.Collect(2,GCCollectionMode.Forced,blocking:false,compacting:false);if(GC.GetGCMemoryInfo().Generation==2&&GC.GetGCMemoryInfo().Concurrent){// Concurrent Gen2 observed while tracker GC callbacks are registered.// This is the targeted repro envelope for dotnet/runtime#110683.}

Co-authored-by: MichalStrehovsky <13110571+MichalStrehovsky@users.noreply.github.com>
CopilotAI requested review from Copilot and removed request for CopilotJuly 10, 2026 03:33
CopilotAI changed the title [WIP] Add regression test for NativeAOT runtime assertion failureAdd deterministic NativeAOT regression smoke test for GC restricted reverse P/Invoke assert pathJul 10, 2026
@dotnet-policy-service

Copy link
Copy Markdown
Contributor

Tagging subscribers to this area: @agocke, @dotnet/ilc-contrib
See info in area-owners.md if you want to be subscribed.

Co-authored-by: MichalStrehovsky <13110571+MichalStrehovsky@users.noreply.github.com>
@MichalStrehovsky

Copy link
Copy Markdown
Member

@copilot now fix the assert failure by weakening the assertion as described in #110683.

Co-authored-by: MichalStrehovsky <13110571+MichalStrehovsky@users.noreply.github.com>
CopilotAI requested review from Copilot and removed request for CopilotJuly 14, 2026 07:40

CopilotAI commented Jul 14, 2026

Copy link
Copy Markdown
ContributorAuthor

@copilot now fix the assert failure by weakening the assertion as described in #110683.

Fixed in 4413efc — the NativeAOT reverse P/Invoke fast path now asserts IsGCSpecial() for DoNotTriggerGc threads instead of requiring ThreadStore::IsTrapThreadsRequested().

CopilotAI 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.

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

CopilotAI review requested due to automatic review settings July 17, 2026 06:14

CopilotAI 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.

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

CopilotAI review requested due to automatic review settings July 21, 2026 08:15

CopilotAI 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.

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

Comments suppressed due to low confidence (1)

src/tests/Interop/COM/ComWrappers/GcRestrictedCalloutReversePInvoke/GcRestrictedCalloutReversePInvoke.csproj:21

  • This project uses xUnit ([Fact] in Program.cs) but unlike the other ComWrappers interop test projects in this directory it doesn’t reference $(TestLibraryProjectPath). As-is, this will likely fail to compile due to missing xUnit/test infrastructure references.
 <ItemGroup>
<Compile Include="../Common.cs" />
<Compile Include="Program.cs" />
</ItemGroup>
<ItemGroup>
<CMakeProjectReference Include="../MockReferenceTrackerRuntime/CMakeLists.txt" />
</ItemGroup>
</Project>

CopilotAI review requested due to automatic review settings July 28, 2026 12:39
@MichalStrehovsky
MichalStrehovsky marked this pull request as ready for review July 28, 2026 12:40
@azure-pipelines

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.

@MichalStrehovsky

Copy link
Copy Markdown
Member

We now have a test that hits the assert without this fix and works with the fix. Is the test valuable enough to keep?

Cc @VSadov

CopilotAI 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.

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (4)

src/tests/Interop/COM/ComWrappers/GcRestrictedCalloutReversePInvoke/GcRestrictedCalloutReversePInvoke.csproj:16

  • This test project uses xUnit ([Fact] in Program.cs) but the csproj is missing the standard that all other ComWrappers test projects in this directory use. Without it, the project may not compile or may miss test infrastructure references.
 <ItemGroup>
<Compile Include="../Common.cs" />
<Compile Include="Program.cs" />
</ItemGroup>

src/tests/Interop/COM/ComWrappers/GcRestrictedCalloutReversePInvoke/GcRestrictedCalloutReversePInvoke.csproj:5

  • This project is described as a NativeAOT smoke test in the PR metadata, but it’s currently added under src/tests/Interop/COM/ComWrappers (not src/tests/nativeaot/SmokeTests). If it’s intended to run in the NativeAOT smoke test legs, it likely needs to live under the nativeaot test tree (or have additional wiring) to be discovered/executed there.
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<CLRTestPriority>0</CLRTestPriority>
<AllowUnsafeBlocks>true</AllowUnsafeBlocks>
<!-- Test interacts with GC collections, also needed for CLRTestTargetUnsupported below -->

src/tests/Interop/COM/ComWrappers/GcRestrictedCalloutReversePInvoke/Program.cs:63

  • Use xUnit assertions for test failures instead of throwing Exception. This keeps failures consistent with the rest of the ComWrappers tests and produces cleaner output in test reports.
 if (!observedConcurrentGen2)
{
throw new Exception($"Timed out after {timeout} waiting for a concurrent Gen2 GC. Initial count: {initialGen2Collections}, final count: {GC.CollectionCount(2)}.");
}

src/coreclr/nativeaot/Runtime/thread.inl:208

  • The PR description states the change is test-only and does not modify product behavior, but this hunk changes a runtime assertion condition in Thread::InlineTryFastReversePInvoke. Please update the PR description/scope accordingly (or split the runtime change into the appropriate fix PR if this PR is meant to be test-only).
 if (IsDoNotTriggerGcSet())
{
// We expect this scenario only when EE is stopped or we're on a GC worker thread.
ASSERT(ThreadStore::IsTrapThreadsRequested() || IsGCSpecial());
// no need to do anything

@agocke
agocke merged commit 55fba98 into mainAug 6, 2026
117 checks passed
@agocke
agocke deleted the copilot/create-focused-regression-test-issue-110683 branch August 6, 2026 18:18
@dotnet-milestone-botdotnet-milestone-botBot added this to the 11.0-rc1 milestone Aug 7, 2026
jkoritzinsky added a commit that referenced this pull request Aug 12, 2026
…require multithreading support (#132176)
`GcRestrictedCalloutReversePInvoke` fails under stress runs because
`ComWrappers.RegisterForTrackerSupport` throws
`PlatformNotSupportedException` under AnyGCStress/AnyJitStress
configurations (introduced by #130459), and the test inherently depends
on background GC/multithreading.
## Changes
- Added `[ActiveIssue]` attributes disabling the test under
`AnyGCStress` and `AnyJitStress` coreclr configurations, via
`CoreClrConfigurationDetection.IsGCStress` and
`CoreClrConfigurationDetection.IsAnyJitStress`.
- Replaced `[Fact]` with `[ConditionalFact(typeof(PlatformDetection),
nameof(PlatformDetection.IsMultithreadingSupported))]` so the test only
runs where multithreading is supported.
<!-- START COPILOT CODING AGENT SUFFIX -->
- Fixes#131989
---------
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: jkoritzinsky <1571408+jkoritzinsky@users.noreply.github.com>
Co-authored-by: Jeremy Koritzinsky <jekoritz@microsoft.com>
Co-authored-by: Jan Kotas <jkotas@microsoft.com>
MichalStrehovsky added a commit that referenced this pull request Aug 31, 2026
Resolves#132939.
I wasn't sure if we want the test in the first place
(#130459 (comment))
and now that it's known to be flaky, the decision is obvious.
---------
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: MichalStrehovsky <13110571+MichalStrehovsky@users.noreply.github.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ASSERT(ThreadStore::IsTrapThreadsRequested()) in Thread::InlineTryFastReversePInvoke

4 participants

@MichalStrehovsky@agocke
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default"); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Add deterministic NativeAOT regression smoke test for GC restricted reverse P/Invoke assert path - #130459

Merged
agocke merged 11 commits into
mainfrom
copilot/create-focused-regression-test-issue-110683
Aug 6, 2026
Merged

Add deterministic NativeAOT regression smoke test for GC restricted reverse P/Invoke assert path#130459
agocke merged 11 commits into
mainfrom
copilot/create-focused-regression-test-issue-110683

Conversation

CopilotAI commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

Resolves#110683.

This adds a focused NativeAOT regression test for the GC restricted callout path where background GC can invoke managed [UnmanagedCallersOnly] callbacks and hit Thread::InlineTryFastReversePInvoke assertion behavior prior to the runtime fix. The change is test-only and is intended to reliably drive the failing pre-fix path, not modify product behavior.

  • Test added

    • src/tests/nativeaot/SmokeTests/GcRestrictedCalloutReversePInvoke/
    • New smoke test project plus program harness.
  • Repro wiring

    • Registers tracker support via ComWrappers.RegisterForTrackerSupport(...).
    • Creates a tracker object through MockReferenceTrackerRuntime so GC restricted tracker callbacks are active (GCStopCollection path).
  • Deterministic triggering

    • Forces repeated concurrent Gen2 collections (GC.Collect(..., blocking:false)).
    • Runs bounded allocation pressure on a background thread.
    • Uses timeout + synchronization (SpinWait + elapsed bound) instead of unbounded probabilistic loops.
  • Scope/guards

    • Configured as process-isolated NativeAOT smoke test.
    • Excludes Apple mobile and Android targets where the required native dependency is not available in this leg.
    • Keeps coverage broad otherwise (not win-x86-only).
  • Expectation documented

    • Source comment references dotnet/runtime#110683 and states expected behavior:
      • release: test should complete,
      • assertion-enabled runtime (before product fix): can fail on the target assertion path.
  • Environment limitation observed while validating command

    • In this environment, Release NativeAOT targeted test build failed due to missing Microsoft.DotNet.ILCompiler.SingleEntry.targets; Checked configuration was used for targeted execution.
GC.Collect(2,GCCollectionMode.Forced,blocking:false,compacting:false);if(GC.GetGCMemoryInfo().Generation==2&&GC.GetGCMemoryInfo().Concurrent){// Concurrent Gen2 observed while tracker GC callbacks are registered.// This is the targeted repro envelope for dotnet/runtime#110683.}

Co-authored-by: MichalStrehovsky <13110571+MichalStrehovsky@users.noreply.github.com>
CopilotAI requested review from Copilot and removed request for CopilotJuly 10, 2026 03:33
CopilotAI changed the title [WIP] Add regression test for NativeAOT runtime assertion failureAdd deterministic NativeAOT regression smoke test for GC restricted reverse P/Invoke assert pathJul 10, 2026
@dotnet-policy-service

Copy link
Copy Markdown
Contributor

Tagging subscribers to this area: @agocke, @dotnet/ilc-contrib
See info in area-owners.md if you want to be subscribed.

Co-authored-by: MichalStrehovsky <13110571+MichalStrehovsky@users.noreply.github.com>
@MichalStrehovsky

Copy link
Copy Markdown
Member

@copilot now fix the assert failure by weakening the assertion as described in #110683.

Co-authored-by: MichalStrehovsky <13110571+MichalStrehovsky@users.noreply.github.com>
CopilotAI requested review from Copilot and removed request for CopilotJuly 14, 2026 07:40

CopilotAI commented Jul 14, 2026

Copy link
Copy Markdown
ContributorAuthor

@copilot now fix the assert failure by weakening the assertion as described in #110683.

Fixed in 4413efc — the NativeAOT reverse P/Invoke fast path now asserts IsGCSpecial() for DoNotTriggerGc threads instead of requiring ThreadStore::IsTrapThreadsRequested().

CopilotAI 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.

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

CopilotAI review requested due to automatic review settings July 17, 2026 06:14

CopilotAI 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.

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

CopilotAI review requested due to automatic review settings July 21, 2026 08:15

CopilotAI 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.

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

Comments suppressed due to low confidence (1)

src/tests/Interop/COM/ComWrappers/GcRestrictedCalloutReversePInvoke/GcRestrictedCalloutReversePInvoke.csproj:21

  • This project uses xUnit ([Fact] in Program.cs) but unlike the other ComWrappers interop test projects in this directory it doesn’t reference $(TestLibraryProjectPath). As-is, this will likely fail to compile due to missing xUnit/test infrastructure references.
 <ItemGroup>
<Compile Include="../Common.cs" />
<Compile Include="Program.cs" />
</ItemGroup>
<ItemGroup>
<CMakeProjectReference Include="../MockReferenceTrackerRuntime/CMakeLists.txt" />
</ItemGroup>
</Project>

CopilotAI review requested due to automatic review settings July 28, 2026 12:39
@MichalStrehovsky
MichalStrehovsky marked this pull request as ready for review July 28, 2026 12:40
@azure-pipelines

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.

@MichalStrehovsky

Copy link
Copy Markdown
Member

We now have a test that hits the assert without this fix and works with the fix. Is the test valuable enough to keep?

Cc @VSadov

CopilotAI 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.

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (4)

src/tests/Interop/COM/ComWrappers/GcRestrictedCalloutReversePInvoke/GcRestrictedCalloutReversePInvoke.csproj:16

  • This test project uses xUnit ([Fact] in Program.cs) but the csproj is missing the standard that all other ComWrappers test projects in this directory use. Without it, the project may not compile or may miss test infrastructure references.
 <ItemGroup>
<Compile Include="../Common.cs" />
<Compile Include="Program.cs" />
</ItemGroup>

src/tests/Interop/COM/ComWrappers/GcRestrictedCalloutReversePInvoke/GcRestrictedCalloutReversePInvoke.csproj:5

  • This project is described as a NativeAOT smoke test in the PR metadata, but it’s currently added under src/tests/Interop/COM/ComWrappers (not src/tests/nativeaot/SmokeTests). If it’s intended to run in the NativeAOT smoke test legs, it likely needs to live under the nativeaot test tree (or have additional wiring) to be discovered/executed there.
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<CLRTestPriority>0</CLRTestPriority>
<AllowUnsafeBlocks>true</AllowUnsafeBlocks>
<!-- Test interacts with GC collections, also needed for CLRTestTargetUnsupported below -->

src/tests/Interop/COM/ComWrappers/GcRestrictedCalloutReversePInvoke/Program.cs:63

  • Use xUnit assertions for test failures instead of throwing Exception. This keeps failures consistent with the rest of the ComWrappers tests and produces cleaner output in test reports.
 if (!observedConcurrentGen2)
{
throw new Exception($"Timed out after {timeout} waiting for a concurrent Gen2 GC. Initial count: {initialGen2Collections}, final count: {GC.CollectionCount(2)}.");
}

src/coreclr/nativeaot/Runtime/thread.inl:208

  • The PR description states the change is test-only and does not modify product behavior, but this hunk changes a runtime assertion condition in Thread::InlineTryFastReversePInvoke. Please update the PR description/scope accordingly (or split the runtime change into the appropriate fix PR if this PR is meant to be test-only).
 if (IsDoNotTriggerGcSet())
{
// We expect this scenario only when EE is stopped or we're on a GC worker thread.
ASSERT(ThreadStore::IsTrapThreadsRequested() || IsGCSpecial());
// no need to do anything

@agocke
agocke merged commit 55fba98 into mainAug 6, 2026
117 checks passed
@agocke
agocke deleted the copilot/create-focused-regression-test-issue-110683 branch August 6, 2026 18:18
@dotnet-milestone-botdotnet-milestone-botBot added this to the 11.0-rc1 milestone Aug 7, 2026
jkoritzinsky added a commit that referenced this pull request Aug 12, 2026
…require multithreading support (#132176)
`GcRestrictedCalloutReversePInvoke` fails under stress runs because
`ComWrappers.RegisterForTrackerSupport` throws
`PlatformNotSupportedException` under AnyGCStress/AnyJitStress
configurations (introduced by #130459), and the test inherently depends
on background GC/multithreading.
## Changes
- Added `[ActiveIssue]` attributes disabling the test under
`AnyGCStress` and `AnyJitStress` coreclr configurations, via
`CoreClrConfigurationDetection.IsGCStress` and
`CoreClrConfigurationDetection.IsAnyJitStress`.
- Replaced `[Fact]` with `[ConditionalFact(typeof(PlatformDetection),
nameof(PlatformDetection.IsMultithreadingSupported))]` so the test only
runs where multithreading is supported.
<!-- START COPILOT CODING AGENT SUFFIX -->
- Fixes#131989
---------
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: jkoritzinsky <1571408+jkoritzinsky@users.noreply.github.com>
Co-authored-by: Jeremy Koritzinsky <jekoritz@microsoft.com>
Co-authored-by: Jan Kotas <jkotas@microsoft.com>
MichalStrehovsky added a commit that referenced this pull request Aug 31, 2026
Resolves#132939.
I wasn't sure if we want the test in the first place
(#130459 (comment))
and now that it's known to be flaky, the decision is obvious.
---------
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: MichalStrehovsky <13110571+MichalStrehovsky@users.noreply.github.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ASSERT(ThreadStore::IsTrapThreadsRequested()) in Thread::InlineTryFastReversePInvoke

4 participants

@MichalStrehovsky@agocke
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers"); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Add deterministic NativeAOT regression smoke test for GC restricted reverse P/Invoke assert path - #130459

Merged
agocke merged 11 commits into
mainfrom
copilot/create-focused-regression-test-issue-110683
Aug 6, 2026
Merged

Add deterministic NativeAOT regression smoke test for GC restricted reverse P/Invoke assert path#130459
agocke merged 11 commits into
mainfrom
copilot/create-focused-regression-test-issue-110683

Conversation

CopilotAI commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

Resolves#110683.

This adds a focused NativeAOT regression test for the GC restricted callout path where background GC can invoke managed [UnmanagedCallersOnly] callbacks and hit Thread::InlineTryFastReversePInvoke assertion behavior prior to the runtime fix. The change is test-only and is intended to reliably drive the failing pre-fix path, not modify product behavior.

  • Test added

    • src/tests/nativeaot/SmokeTests/GcRestrictedCalloutReversePInvoke/
    • New smoke test project plus program harness.
  • Repro wiring

    • Registers tracker support via ComWrappers.RegisterForTrackerSupport(...).
    • Creates a tracker object through MockReferenceTrackerRuntime so GC restricted tracker callbacks are active (GCStopCollection path).
  • Deterministic triggering

    • Forces repeated concurrent Gen2 collections (GC.Collect(..., blocking:false)).
    • Runs bounded allocation pressure on a background thread.
    • Uses timeout + synchronization (SpinWait + elapsed bound) instead of unbounded probabilistic loops.
  • Scope/guards

    • Configured as process-isolated NativeAOT smoke test.
    • Excludes Apple mobile and Android targets where the required native dependency is not available in this leg.
    • Keeps coverage broad otherwise (not win-x86-only).
  • Expectation documented

    • Source comment references dotnet/runtime#110683 and states expected behavior:
      • release: test should complete,
      • assertion-enabled runtime (before product fix): can fail on the target assertion path.
  • Environment limitation observed while validating command

    • In this environment, Release NativeAOT targeted test build failed due to missing Microsoft.DotNet.ILCompiler.SingleEntry.targets; Checked configuration was used for targeted execution.
GC.Collect(2,GCCollectionMode.Forced,blocking:false,compacting:false);if(GC.GetGCMemoryInfo().Generation==2&&GC.GetGCMemoryInfo().Concurrent){// Concurrent Gen2 observed while tracker GC callbacks are registered.// This is the targeted repro envelope for dotnet/runtime#110683.}

Co-authored-by: MichalStrehovsky <13110571+MichalStrehovsky@users.noreply.github.com>
CopilotAI requested review from Copilot and removed request for CopilotJuly 10, 2026 03:33
CopilotAI changed the title [WIP] Add regression test for NativeAOT runtime assertion failureAdd deterministic NativeAOT regression smoke test for GC restricted reverse P/Invoke assert pathJul 10, 2026
@dotnet-policy-service

Copy link
Copy Markdown
Contributor

Tagging subscribers to this area: @agocke, @dotnet/ilc-contrib
See info in area-owners.md if you want to be subscribed.

Co-authored-by: MichalStrehovsky <13110571+MichalStrehovsky@users.noreply.github.com>
@MichalStrehovsky

Copy link
Copy Markdown
Member

@copilot now fix the assert failure by weakening the assertion as described in #110683.

Co-authored-by: MichalStrehovsky <13110571+MichalStrehovsky@users.noreply.github.com>
CopilotAI requested review from Copilot and removed request for CopilotJuly 14, 2026 07:40

CopilotAI commented Jul 14, 2026

Copy link
Copy Markdown
ContributorAuthor

@copilot now fix the assert failure by weakening the assertion as described in #110683.

Fixed in 4413efc — the NativeAOT reverse P/Invoke fast path now asserts IsGCSpecial() for DoNotTriggerGc threads instead of requiring ThreadStore::IsTrapThreadsRequested().

CopilotAI 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.

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

CopilotAI review requested due to automatic review settings July 17, 2026 06:14

CopilotAI 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.

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

CopilotAI review requested due to automatic review settings July 21, 2026 08:15

CopilotAI 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.

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

Comments suppressed due to low confidence (1)

src/tests/Interop/COM/ComWrappers/GcRestrictedCalloutReversePInvoke/GcRestrictedCalloutReversePInvoke.csproj:21

  • This project uses xUnit ([Fact] in Program.cs) but unlike the other ComWrappers interop test projects in this directory it doesn’t reference $(TestLibraryProjectPath). As-is, this will likely fail to compile due to missing xUnit/test infrastructure references.
 <ItemGroup>
<Compile Include="../Common.cs" />
<Compile Include="Program.cs" />
</ItemGroup>
<ItemGroup>
<CMakeProjectReference Include="../MockReferenceTrackerRuntime/CMakeLists.txt" />
</ItemGroup>
</Project>

CopilotAI review requested due to automatic review settings July 28, 2026 12:39
@MichalStrehovsky
MichalStrehovsky marked this pull request as ready for review July 28, 2026 12:40
@azure-pipelines

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.

@MichalStrehovsky

Copy link
Copy Markdown
Member

We now have a test that hits the assert without this fix and works with the fix. Is the test valuable enough to keep?

Cc @VSadov

CopilotAI 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.

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (4)

src/tests/Interop/COM/ComWrappers/GcRestrictedCalloutReversePInvoke/GcRestrictedCalloutReversePInvoke.csproj:16

  • This test project uses xUnit ([Fact] in Program.cs) but the csproj is missing the standard that all other ComWrappers test projects in this directory use. Without it, the project may not compile or may miss test infrastructure references.
 <ItemGroup>
<Compile Include="../Common.cs" />
<Compile Include="Program.cs" />
</ItemGroup>

src/tests/Interop/COM/ComWrappers/GcRestrictedCalloutReversePInvoke/GcRestrictedCalloutReversePInvoke.csproj:5

  • This project is described as a NativeAOT smoke test in the PR metadata, but it’s currently added under src/tests/Interop/COM/ComWrappers (not src/tests/nativeaot/SmokeTests). If it’s intended to run in the NativeAOT smoke test legs, it likely needs to live under the nativeaot test tree (or have additional wiring) to be discovered/executed there.
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<CLRTestPriority>0</CLRTestPriority>
<AllowUnsafeBlocks>true</AllowUnsafeBlocks>
<!-- Test interacts with GC collections, also needed for CLRTestTargetUnsupported below -->

src/tests/Interop/COM/ComWrappers/GcRestrictedCalloutReversePInvoke/Program.cs:63

  • Use xUnit assertions for test failures instead of throwing Exception. This keeps failures consistent with the rest of the ComWrappers tests and produces cleaner output in test reports.
 if (!observedConcurrentGen2)
{
throw new Exception($"Timed out after {timeout} waiting for a concurrent Gen2 GC. Initial count: {initialGen2Collections}, final count: {GC.CollectionCount(2)}.");
}

src/coreclr/nativeaot/Runtime/thread.inl:208

  • The PR description states the change is test-only and does not modify product behavior, but this hunk changes a runtime assertion condition in Thread::InlineTryFastReversePInvoke. Please update the PR description/scope accordingly (or split the runtime change into the appropriate fix PR if this PR is meant to be test-only).
 if (IsDoNotTriggerGcSet())
{
// We expect this scenario only when EE is stopped or we're on a GC worker thread.
ASSERT(ThreadStore::IsTrapThreadsRequested() || IsGCSpecial());
// no need to do anything

@agocke
agocke merged commit 55fba98 into mainAug 6, 2026
117 checks passed
@agocke
agocke deleted the copilot/create-focused-regression-test-issue-110683 branch August 6, 2026 18:18
@dotnet-milestone-botdotnet-milestone-botBot added this to the 11.0-rc1 milestone Aug 7, 2026
jkoritzinsky added a commit that referenced this pull request Aug 12, 2026
…require multithreading support (#132176)
`GcRestrictedCalloutReversePInvoke` fails under stress runs because
`ComWrappers.RegisterForTrackerSupport` throws
`PlatformNotSupportedException` under AnyGCStress/AnyJitStress
configurations (introduced by #130459), and the test inherently depends
on background GC/multithreading.
## Changes
- Added `[ActiveIssue]` attributes disabling the test under
`AnyGCStress` and `AnyJitStress` coreclr configurations, via
`CoreClrConfigurationDetection.IsGCStress` and
`CoreClrConfigurationDetection.IsAnyJitStress`.
- Replaced `[Fact]` with `[ConditionalFact(typeof(PlatformDetection),
nameof(PlatformDetection.IsMultithreadingSupported))]` so the test only
runs where multithreading is supported.
<!-- START COPILOT CODING AGENT SUFFIX -->
- Fixes#131989
---------
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: jkoritzinsky <1571408+jkoritzinsky@users.noreply.github.com>
Co-authored-by: Jeremy Koritzinsky <jekoritz@microsoft.com>
Co-authored-by: Jan Kotas <jkotas@microsoft.com>
MichalStrehovsky added a commit that referenced this pull request Aug 31, 2026
Resolves#132939.
I wasn't sure if we want the test in the first place
(#130459 (comment))
and now that it's known to be flaky, the decision is obvious.
---------
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: MichalStrehovsky <13110571+MichalStrehovsky@users.noreply.github.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ASSERT(ThreadStore::IsTrapThreadsRequested()) in Thread::InlineTryFastReversePInvoke

4 participants

@MichalStrehovsky@agocke
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode"); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })();
Skip to content

Add deterministic NativeAOT regression smoke test for GC restricted reverse P/Invoke assert path - #130459

Merged
agocke merged 11 commits into
mainfrom
copilot/create-focused-regression-test-issue-110683
Aug 6, 2026
Merged

Add deterministic NativeAOT regression smoke test for GC restricted reverse P/Invoke assert path#130459
agocke merged 11 commits into
mainfrom
copilot/create-focused-regression-test-issue-110683

Conversation

CopilotAI commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

Resolves#110683.

This adds a focused NativeAOT regression test for the GC restricted callout path where background GC can invoke managed [UnmanagedCallersOnly] callbacks and hit Thread::InlineTryFastReversePInvoke assertion behavior prior to the runtime fix. The change is test-only and is intended to reliably drive the failing pre-fix path, not modify product behavior.

  • Test added

    • src/tests/nativeaot/SmokeTests/GcRestrictedCalloutReversePInvoke/
    • New smoke test project plus program harness.
  • Repro wiring

    • Registers tracker support via ComWrappers.RegisterForTrackerSupport(...).
    • Creates a tracker object through MockReferenceTrackerRuntime so GC restricted tracker callbacks are active (GCStopCollection path).
  • Deterministic triggering

    • Forces repeated concurrent Gen2 collections (GC.Collect(..., blocking:false)).
    • Runs bounded allocation pressure on a background thread.
    • Uses timeout + synchronization (SpinWait + elapsed bound) instead of unbounded probabilistic loops.
  • Scope/guards

    • Configured as process-isolated NativeAOT smoke test.
    • Excludes Apple mobile and Android targets where the required native dependency is not available in this leg.
    • Keeps coverage broad otherwise (not win-x86-only).
  • Expectation documented

    • Source comment references dotnet/runtime#110683 and states expected behavior:
      • release: test should complete,
      • assertion-enabled runtime (before product fix): can fail on the target assertion path.
  • Environment limitation observed while validating command

    • In this environment, Release NativeAOT targeted test build failed due to missing Microsoft.DotNet.ILCompiler.SingleEntry.targets; Checked configuration was used for targeted execution.
GC.Collect(2,GCCollectionMode.Forced,blocking:false,compacting:false);if(GC.GetGCMemoryInfo().Generation==2&&GC.GetGCMemoryInfo().Concurrent){// Concurrent Gen2 observed while tracker GC callbacks are registered.// This is the targeted repro envelope for dotnet/runtime#110683.}

Co-authored-by: MichalStrehovsky <13110571+MichalStrehovsky@users.noreply.github.com>
CopilotAI requested review from Copilot and removed request for CopilotJuly 10, 2026 03:33
CopilotAI changed the title [WIP] Add regression test for NativeAOT runtime assertion failureAdd deterministic NativeAOT regression smoke test for GC restricted reverse P/Invoke assert pathJul 10, 2026
@dotnet-policy-service

Copy link
Copy Markdown
Contributor

Tagging subscribers to this area: @agocke, @dotnet/ilc-contrib
See info in area-owners.md if you want to be subscribed.

Co-authored-by: MichalStrehovsky <13110571+MichalStrehovsky@users.noreply.github.com>
@MichalStrehovsky

Copy link
Copy Markdown
Member

@copilot now fix the assert failure by weakening the assertion as described in #110683.

Co-authored-by: MichalStrehovsky <13110571+MichalStrehovsky@users.noreply.github.com>
CopilotAI requested review from Copilot and removed request for CopilotJuly 14, 2026 07:40

CopilotAI commented Jul 14, 2026

Copy link
Copy Markdown
ContributorAuthor

@copilot now fix the assert failure by weakening the assertion as described in #110683.

Fixed in 4413efc — the NativeAOT reverse P/Invoke fast path now asserts IsGCSpecial() for DoNotTriggerGc threads instead of requiring ThreadStore::IsTrapThreadsRequested().

CopilotAI 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.

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

CopilotAI review requested due to automatic review settings July 17, 2026 06:14

CopilotAI 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.

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

CopilotAI review requested due to automatic review settings July 21, 2026 08:15

CopilotAI 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.

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

Comments suppressed due to low confidence (1)

src/tests/Interop/COM/ComWrappers/GcRestrictedCalloutReversePInvoke/GcRestrictedCalloutReversePInvoke.csproj:21

  • This project uses xUnit ([Fact] in Program.cs) but unlike the other ComWrappers interop test projects in this directory it doesn’t reference $(TestLibraryProjectPath). As-is, this will likely fail to compile due to missing xUnit/test infrastructure references.
 <ItemGroup>
<Compile Include="../Common.cs" />
<Compile Include="Program.cs" />
</ItemGroup>
<ItemGroup>
<CMakeProjectReference Include="../MockReferenceTrackerRuntime/CMakeLists.txt" />
</ItemGroup>
</Project>

CopilotAI review requested due to automatic review settings July 28, 2026 12:39
@MichalStrehovsky
MichalStrehovsky marked this pull request as ready for review July 28, 2026 12:40
@azure-pipelines

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.

@MichalStrehovsky

Copy link
Copy Markdown
Member

We now have a test that hits the assert without this fix and works with the fix. Is the test valuable enough to keep?

Cc @VSadov

CopilotAI 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.

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (4)

src/tests/Interop/COM/ComWrappers/GcRestrictedCalloutReversePInvoke/GcRestrictedCalloutReversePInvoke.csproj:16

  • This test project uses xUnit ([Fact] in Program.cs) but the csproj is missing the standard that all other ComWrappers test projects in this directory use. Without it, the project may not compile or may miss test infrastructure references.
 <ItemGroup>
<Compile Include="../Common.cs" />
<Compile Include="Program.cs" />
</ItemGroup>

src/tests/Interop/COM/ComWrappers/GcRestrictedCalloutReversePInvoke/GcRestrictedCalloutReversePInvoke.csproj:5

  • This project is described as a NativeAOT smoke test in the PR metadata, but it’s currently added under src/tests/Interop/COM/ComWrappers (not src/tests/nativeaot/SmokeTests). If it’s intended to run in the NativeAOT smoke test legs, it likely needs to live under the nativeaot test tree (or have additional wiring) to be discovered/executed there.
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<CLRTestPriority>0</CLRTestPriority>
<AllowUnsafeBlocks>true</AllowUnsafeBlocks>
<!-- Test interacts with GC collections, also needed for CLRTestTargetUnsupported below -->

src/tests/Interop/COM/ComWrappers/GcRestrictedCalloutReversePInvoke/Program.cs:63

  • Use xUnit assertions for test failures instead of throwing Exception. This keeps failures consistent with the rest of the ComWrappers tests and produces cleaner output in test reports.
 if (!observedConcurrentGen2)
{
throw new Exception($"Timed out after {timeout} waiting for a concurrent Gen2 GC. Initial count: {initialGen2Collections}, final count: {GC.CollectionCount(2)}.");
}

src/coreclr/nativeaot/Runtime/thread.inl:208

  • The PR description states the change is test-only and does not modify product behavior, but this hunk changes a runtime assertion condition in Thread::InlineTryFastReversePInvoke. Please update the PR description/scope accordingly (or split the runtime change into the appropriate fix PR if this PR is meant to be test-only).
 if (IsDoNotTriggerGcSet())
{
// We expect this scenario only when EE is stopped or we're on a GC worker thread.
ASSERT(ThreadStore::IsTrapThreadsRequested() || IsGCSpecial());
// no need to do anything

@agocke
agocke merged commit 55fba98 into mainAug 6, 2026
117 checks passed
@agocke
agocke deleted the copilot/create-focused-regression-test-issue-110683 branch August 6, 2026 18:18
@dotnet-milestone-botdotnet-milestone-botBot added this to the 11.0-rc1 milestone Aug 7, 2026
jkoritzinsky added a commit that referenced this pull request Aug 12, 2026
…require multithreading support (#132176)
`GcRestrictedCalloutReversePInvoke` fails under stress runs because
`ComWrappers.RegisterForTrackerSupport` throws
`PlatformNotSupportedException` under AnyGCStress/AnyJitStress
configurations (introduced by #130459), and the test inherently depends
on background GC/multithreading.
## Changes
- Added `[ActiveIssue]` attributes disabling the test under
`AnyGCStress` and `AnyJitStress` coreclr configurations, via
`CoreClrConfigurationDetection.IsGCStress` and
`CoreClrConfigurationDetection.IsAnyJitStress`.
- Replaced `[Fact]` with `[ConditionalFact(typeof(PlatformDetection),
nameof(PlatformDetection.IsMultithreadingSupported))]` so the test only
runs where multithreading is supported.
<!-- START COPILOT CODING AGENT SUFFIX -->
- Fixes#131989
---------
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: jkoritzinsky <1571408+jkoritzinsky@users.noreply.github.com>
Co-authored-by: Jeremy Koritzinsky <jekoritz@microsoft.com>
Co-authored-by: Jan Kotas <jkotas@microsoft.com>
MichalStrehovsky added a commit that referenced this pull request Aug 31, 2026
Resolves#132939.
I wasn't sure if we want the test in the first place
(#130459 (comment))
and now that it's known to be flaky, the decision is obvious.
---------
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: MichalStrehovsky <13110571+MichalStrehovsky@users.noreply.github.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ASSERT(ThreadStore::IsTrapThreadsRequested()) in Thread::InlineTryFastReversePInvoke

4 participants

@MichalStrehovsky@agocke