Skip to content

fix(test): the inline-resumption probe compared thread IDs, which proves nothing - #2788

Closed
rbuergi wants to merge 1 commit into
mainfrom
fix/inline-resumption-probe-is-sound
Closed

rbuergi wants to merge 1 commit into
mainfrom
fix/inline-resumption-probe-is-sound

Conversation

@rbuergi

@rbuergi rbuergi commented Aug 30, 2026

Copy link
Copy Markdown
Contributor

The intermittent

InlineResumptionMechanismTest.ObserveCompletion_DoesNotResumeOnTheSignallingThread failed on #2781 — a documentation-only PR (a Homebrew formula, a README, one doc page). Nothing in that diff can reach a threading primitive.

Why it fails, and why re-running would have buried it

It asserted NotEqual(signalling, resumed) on Environment.CurrentManagedThreadId. That proves nothing in the negative direction: a correctly queued continuation runs on a pool thread, and the pool is free to hand it the very thread that just finished signalling. The assertion fails while the behaviour under test is entirely correct.

🚨 The same unsound shape is already written down two files away, in ActivityLiveProgressTest:

"createdThread != commandThread — the runtime makes no such [guarantee]"

Rediscovered rather than reused. This is my own test family from the ToTask sweep, so the miss is mine.

What it measures now

The continuation ran on the signalling thread AND while the signaller was still inside OnNext/OnCompleted.

Neither half suffices, and I proved both insufficient rather than assuming:

probe verdict
thread ids differ the intermittent this replaces
"the waiter had completed when OnCompleted returned" my first attempt — reported INLINE on 5 of 5 runs against ObserveCompletion, because the pool runs a queued continuation inside that window and measurably does
same thread and inside the call rigorous

Together they are rigorous because a thread cannot be in two places at once: if the continuation ran on the signalling thread while that thread was still inside the signalling call, it was on the signaller's stack. A queued continuation that later lands on that recycled thread necessarily finds the flag cleared — the thread was occupied until the signaller left.

The mutation proof is built in

The two positive controls — Rx's .ToTask() bridge, and awaiting the observable directly — must report TRUE. They are the negative controls for this measurement: if it stopped discriminating, they would go red. So the proof lives in the file rather than being performed once and forgotten.

Verified: 6 consecutive runs, 3/3 green each.

Note the two positive assertions were always sound — inline resumption does imply the same thread — so only the negative direction needed replacing.

🤖 Generated with Claude Code

…ves nothing

InlineResumptionMechanismTest.ObserveCompletion_DoesNotResumeOnTheSignalling
Thread asserted `NotEqual(signalling, resumed)` on
Environment.CurrentManagedThreadId. That is unsound in the negative direction:
a correctly QUEUED continuation runs on a pool thread, and the pool is free to
hand it the very thread that just finished signalling. So the assertion fails
intermittently while the behaviour under test is perfectly correct — observed
2026-08-30 on #2781, a DOCUMENTATION-ONLY PR (a Homebrew formula, a README and
one doc page) that cannot reach a threading primitive.

🚨 The same unsound shape is already recorded two files away, in
ActivityLiveProgressTest: "createdThread != commandThread — the runtime makes
no such [guarantee]". Rediscovered rather than reused.

The two positive controls were always sound — inline resumption DOES imply the
same thread — so only the negative direction needed replacing.

What it measures now: the continuation ran on the signalling thread AND while
the signaller was still inside OnNext/OnCompleted. Neither half suffices, and I
proved both insufficient rather than assuming:

  * thread ids differing — the intermittent this replaces;
  * "the waiter had completed when OnCompleted returned" — my first attempt,
    which reported INLINE on 5 of 5 runs against ObserveCompletion, because the
    pool can run a queued continuation inside that window and measurably does.

Together they are rigorous: a thread cannot be in two places at once. If the
continuation ran on the signalling thread while that thread was still inside
the signalling call, it was on the signaller's stack. A queued continuation
later landing on that recycled thread necessarily finds the flag cleared,
because the thread was occupied until the signaller left.

The two positive controls (Rx's bridge; awaiting the observable directly) must
report TRUE, so they prove the measurement still discriminates — the
mutation-proof is built into the file rather than performed once.

Verified: 6 consecutive runs, 3/3 green each.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Copilot AI lite review requested due to automatic review settings August 30, 2026 14:36

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR strengthens InlineResumptionMechanismTest by replacing an unsound “different thread ID” negative assertion with a more rigorous probe for inline continuation resumption (same thread and still inside the signalling call), preventing intermittent failures under load while preserving the intent of the test.

Changes:

  • Reworks the measurement helper to detect inline resumption via an “inside signalling” flag rather than relying on managed thread ID inequality.
  • Updates assertions to use ResumedInline (with improved failure messages) for the positive and negative controls.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment on lines 70 to 73
// Let the waiter subscribe before we signal. A bare Subject drops a notification that
// arrives with no observer attached, so the wait would never complete.
await Task.Delay(300, TestContext.Current.CancellationToken);

@github-actions

Copy link
Copy Markdown
Contributor

Test Results (shard 3)

730 tests   538 ✅  4m 59s ⏱️
  5 suites  192 💤
  5 files      0 ❌

Results for commit 30456cb.

@github-actions

Copy link
Copy Markdown
Contributor

Test Results (shard 0)

951 tests   951 ✅  5m 17s ⏱️
  6 suites    0 💤
  6 files      0 ❌

Results for commit 30456cb.

@github-actions

Copy link
Copy Markdown
Contributor

Test Results (shard 5)

1 547 tests   1 547 ✅  6m 30s ⏱️
    8 suites      0 💤
    8 files        0 ❌

Results for commit 30456cb.

@github-actions

Copy link
Copy Markdown
Contributor

Test Results (shard 4)

   10 files     10 suites   5m 13s ⏱️
3 481 tests 3 481 ✅ 0 💤 0 ❌
4 145 runs  4 145 ✅ 0 💤 0 ❌

Results for commit 30456cb.

@github-actions

Copy link
Copy Markdown
Contributor

Test Results (shard 1)

971 tests   967 ✅  5m 52s ⏱️
  6 suites    4 💤
  6 files      0 ❌

Results for commit 30456cb.

@github-actions

Copy link
Copy Markdown
Contributor

Test Results (shard 2)

803 tests   803 ✅  6m 54s ⏱️
  6 suites    0 💤
  6 files      0 ❌

Results for commit 30456cb.

@github-actions

Copy link
Copy Markdown
Contributor

Test Results

   41 files     41 suites   34m 46s ⏱️
8 483 tests 8 287 ✅ 196 💤 0 ❌
9 147 runs  8 951 ✅ 196 💤 0 ❌

Results for commit 30456cb.

@rbuergi

rbuergi commented Aug 30, 2026

Copy link
Copy Markdown
Contributor Author

Superseded by #2791, which landed first and is sound. Closing rather than forcing a competing rewrite of the same test — that would be churn on a file someone has just fixed.

#2791 removes the confound instead of measuring around it: the signal is sent from a dedicated Thread, not a pool worker. A live thread's managed ID is unique, so a queued continuation running on a pool thread can never share it — which makes NotEqual(signalling, resumed) sound in the direction that was previously unsound, while the two Equal assertions keep holding for the inline cases. Simpler than what I had.

Third time today I have duplicated a sibling's work (also #2773/#2772 and #2778/#2779). The tell each time was the same and I keep noticing it too late: a conflict in a file I did not expect to be contested.

Worth keeping from this branch's investigation, since the reasoning is not obvious and the next person may reach for one of these:

  • Comparing managed thread IDs is unsound in the negative direction unless the signaller is off-pool — the pool may hand a queued continuation the very thread that just finished signalling. That is what test: InlineResumptionMechanismTest signals from a dedicated thread, not a pool worker #2791 fixes by construction.
  • "The waiter had completed by the time OnCompleted returned" is also unsound. I tried it as the replacement and measured it: it reported inline on 5 of 5 runs against ObserveCompletion, because the pool can and does run a queued continuation inside that window.
  • The rigorous direct form, if a future change ever removes the dedicated producer thread, is same thread AND still inside the signalling call — a thread cannot be in two places at once, so that combination can only mean the continuation ran on the signaller's stack.

The file's two positive controls (Rx's bridge, and awaiting the observable directly) remain the thing that keeps any of these honest: if the measurement stopped discriminating, they would go green when they must be red.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants