[thomhurst]

Fixes #7554.

Diagnosis

The Windows [EngineTest=Pass] reflection run is slow, not hung. I analyzed the raw hang dumps from runs 41849691180 (the one #7584 analyzed) and 41860407100 (the latest #7584 head) locally with dotnet-dump, plus the minidump thread and process CPU times:

Dump	Wall time at capture	Process CPU	Progress at capture (local TRX offset of running tests)
41849691180	8m	22 s	~17% (dynamic tests)
41860407100	8m	101 s (52 s kernel)	~86% (final NotInParallel phase, _6885 Test3 just started)

A complete local run uses about 140 s of CPU, so in both dumps the CPU time matches the progress. The newest dump has no blocked chain: dumpasync shows only DedicatedThreadExecutorCleanUpOrderTests.Test3 awaiting its Task.Delay(400). The two long-running names in the hang log, Run and InstanceMethodDataSource_DoesNotReuseEnumerationInstance, are deferred placeholders that are reported as running for the whole session (see the issue thread). The Thread.StartInternal/PulseAllHelper pairs in the older dump are threads starting and exiting under CPU starvation (the machine was at 100%), not a deadlock: every target thread had already run its body (_startHelper is null and TS_Unstarted is cleared).

A passing Windows run already takes 7m12s (run 41856853804), against the 8-minute dump and the 10-minute timeout. Linux and macOS finish in 2 to 4 minutes.

Cause

STAThreadTests is Windows-only and has [Repeat(100)], which gives 1,515 tests. STAThreadExecutor also runs every hook for those tests, and the project has about 55 global [BeforeEvery(Test)]/[AfterEvery(Test)] hooks. Each test and each hook runs on its own new STA thread, so this class starts about 78,000 STA threads. A local harness measured about 1.2 ms of CPU (mostly kernel) for each STA execution.

Measured on 4 pinned cores locally (DOTNET_PROCESSOR_COUNT=4, affinity 0xF), with the Pass filter in reflection mode:

	CPU (user/kernel)	Wall
main	63 s / 77 s	2m33s
without STAThreadTests	13 s / 3 s	1m38s
[Repeat(10)] (this PR)	19 s / 11 s	1m48s

On the shared Windows runner this run gets only part of the CPU, so cutting the CPU it needs by about 4.6x cuts its wall time by about the same factor.

Change

STAThreadTests changes from [Repeat(100)] to [Repeat(10)]. Every STA scenario still runs 11 times, and the session timeout is unchanged. A comment records why the count must stay low.

Validation

• STAThreadTests passes with 165 tests in both source-generated and reflection modes (--treenode-filter /*/*/STAThreadTests/*).
• The full /**[EngineTest=Pass] reflection run on 4 local cores passes with 5,417 tests.
• I did not run TUnit.Engine.Tests locally. The Windows engine job in CI checks the full session.

Out of scope

I also tried two other changes. Neither made a measurable difference, so neither is included:

• removing SpinWait.SpinOnce (which calls Sleep(1)) from the DedicatedThreadExecutor pump
• replacing the global HookGeneratorTest.AfterEachTest Task.Delay(1) with Task.Yield()

Summary by CodeRabbit

• Tests
  • Reduced the number of repetitions for STA-thread test cases from 100 to 10, shortening the test run. This change applies to the test suite; it does not alter application functionality or introduce changes to the public interface.

[chatgpt-codex-connector]

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-11-04T17:08:23.375798Z	65df0aa	PR opened

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

[coderabbitai]

No actionable comments were generated in the recent review.

Walkthrough

The STAThreadTests repeat count changes from 100 to 10. Comments state that each repetition runs the test and global before/after hooks on new STA threads, and that 100 repetitions caused the Windows CI run to exceed its timeout.

Changes: STA test repetition adjustment

Layer / File(s)	Summary
Adjust STA test repetitions tests/TUnit.TestProject/STAThreadTests.cs	The repeat count changes from 100 to 10. Comments describe the thread and CPU costs per repetition and report that 100 repetitions exceeded the Windows CI timeout.

Estimated code review effort: 2 (Simple) | ~5 minutes

Change: Bug fix · Severity of issue fixed: Medium

Possibly related issues

• Issue 7359: The repeat-count reduction lowers CPU time in the Windows reflection-mode run, which relates to the issue’s CI-duration concern.

Possibly related PRs

• thomhurst/TUnit#7565: Adds a hang dump for the same Windows reflection-mode test run.
• thomhurst/TUnit#7584: Analyzes hang dumps from the same Windows engine test run, including evidence about STA thread creation.

Merge Risk: Minimal · up to 65df0

No review-blocking correctness or availability risk is identified; the change is suitable to merge with normal CI validation.

Pre-merge checks: 4 · 1

Failed checks: 1 warning

Check name	Status	Explanation	Resolution
Linked Issues check		Scope: directly linked issue #7554 is active. Its coding requirements are: (1) identify the fully qualified tests and the execution path that stall the session; (2) fix the underlying cause without ra…	1. Add focused regression coverage for the diagnosed cause. For example, add a test that verifies STA execution does not start a new thread for each global hook, or that bounds the STA thread count for a test run. 2. Address the per-executi…

Passed checks: 4 passed

Check name	Status	Explanation
Description Check		Check skipped - CodeRabbit’s high-level summary is enabled.
Title check		The title clearly and concisely describes the main change: reducing STAThreadTests repetitions to fit the Windows reflection-mode run within its timeout.
Out of Scope Changes check		The reviewed diff from merge base to head changes one file: tests/TUnit.TestProject/STAThreadTests.cs. It changes [Repeat(100)] to [Repeat(10)] and adds a comment that explains the thread cost. Both c…
Docstring Coverage		No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat…

Full details: Out of Scope Changes check

Explanation

The reviewed diff from merge base to head changes one file: tests/TUnit.TestProject/STAThreadTests.cs. It changes [Repeat(100)] to [Repeat(10)] and adds a comment that explains the thread cost. Both changes relate directly to the diagnosed cause in #7554. The PR description lists the SpinWait.SpinOnce removal and the Task.Delay(1) replacement as tested but not included. The diff contains neither change, so no unrelated changes are present.

Full details: Docstring Coverage

Explanation

No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 0 functions across 1 files.

Full details: Linked Issues check

Explanation

Scope: directly linked issue #7554 is active. Its coding requirements are: (1) identify the fully qualified tests and the execution path that stall the session; (2) fix the underlying cause without raising the timeout or skipping tests; (3) add focused regression coverage for the diagnosed cause and validate the full Windows engine session in CI.

Requirement 1, partly met: The PR gives a diagnosis. It attributes the slowdown to STA thread creation in STAThreadTests, where STAThreadExecutor runs each test and each global hook on a new STA thread. The diff confirms the comment that describes this. The PR also states that Run and InstanceMethodDataSource_DoesNotReuseEnumerationInstance are deferred placeholders. It does not name the Run test class. It does not show the Issue6361 class in Bugs/6361/Tests.cs as the source of the placeholder. The dump evidence is not in the repository, so it cannot be verified here.

Requirement 2, partly met: The timeout is unchanged and no test is skipped, as the diff shows. The change lowers the repeat count from 100 to 10 in STAThreadTests.cs. This reduces load. It does not change STAThreadExecutor, so the per-execution thread creation that the PR identifies remains. Whether this counts as fixing the underlying cause is open.

Requirement 3, not met: The diff changes only the Repeat attribute and adds a comment in tests/TUnit.TestProject/STAThreadTests.cs. It adds no regression test for the diagnosed cause, such as a check on STA thread count per execution or on hook execution. Full-session CI validation is deferred to the Windows engine job and has not been shown.

Resolution

1. Add focused regression coverage for the diagnosed cause. For example, add a test that verifies STA execution does not start a new thread for each global hook, or that bounds the STA thread count for a test run.
2. Address the per-execution STA thread creation in STAThreadExecutor, or document why lowering the repeat count is the chosen fix and why it is sufficient.
3. Identify the fully qualified test named Run that remains active in the hang log, or state that it cannot be identified from the available evidence.
4. Provide the Windows engine CI run result for the full session with this change.

Finishing Touches

Generate docstrings

• Commit to this branch
• Create a new PR

Generate unit tests (beta)

• Commit to this branch
• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

Share

• X
• Mastodon
• Reddit
• LinkedIn

_{A rabbit counts each thread with care,
Ten hops replace a hundred there.
The hooks still run on STA ground,
Less waiting fills the test-run sound.
The bunny files its green report.}

Recent review info

Run configuration

• Source mode: Auto
• Repository: Unavailable
• Branch: Unavailable
• File: No configuration file from this repository applied
• Commit: Unavailable
• Layers: Organization UI
• Review profile: CHILL
• Plan: Advanced
• Run ID: 766de24a-d9fc-4d6a-a897-5044493ba7dc

Commits

Reviewing files that changed from the base of the PR and between 69d3960 and 65df0aa.

Files selected for processing: 1

• tests/TUnit.TestProject/STAThreadTests.cs

Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 1 remain after this review.

[greptile-apps]

Confidence Score: 5/5

[Low impact] This PR appears safe to merge.

Summary

Reduces STAThreadTests from 100 repeats to 10 to cut Windows CI thread overhead.

• The Windows pass run repeats each STA test scenario 11 times.

_{Reviews (1) · Last reviewed commit: "Cut STAThreadTests repeats so the Window..." · Reviewed by Greptile}

[github-actions]

Review: LGTM. One-line change reducing STAThreadTests [Repeat(100)] to [Repeat(10)], with a comment explaining why the count must stay low. The diagnosis in the description is well-supported: the thread-per-hook STA cost scales with the repeat count, and the measured CPU drop (~4.6x) addresses the Windows timeout (#7554). Every STA scenario still runs 11 times, so coverage of the behaviour is retained.

Optional, non-blocking: the root cause is that each STA execution spawns a new thread for every global hook. A follow-up could reuse a single STA thread per test (running its hooks on it) in STAThreadExecutor. That would cut thread churn for all users, not just this test, and would let the repeat count go back up. This is out of scope for this PR.