Skip to content

fix: the two CI flakes on main (net10 test host crash, lost Monitor.Pulse) - #14

Merged
IvanMurzak merged 4 commits into
mainfrom
fix/ci-flakes
Nov 6, 2026
Merged

IvanMurzak merged 4 commits into
mainfrom
fix/ci-flakes

Conversation

@IvanMurzak

Copy link
Copy Markdown
Owner

main CI was red on the last two pushes (dotnet (Roslyn 4.0.1)). A stress run (full suite ×40, then ×120) found two separate flakes.

1. Test host crash on .NET 10 (Test host process crashed, no stderr)

  • Caught with --blame-crash; the dump, symbolized with the runtime's symbols on a Linux runner: SIGSEGV in the GC, CFinalize::ScanForFinalization → mark_object_simple1 → MethodTable::GetFlag(this=0x0), i.e. an object reachable from the finalization queue whose child has no method table. The test thread was compiling a snippet at the time, not running the snippet's recursion.
  • Suspect (not proven, a standalone repro did not crash): the guard's [ThreadStatic] static Run _thread lives in the collectible snippet assembly; .NET 9+ reworked thread statics, net8 never crashed. The guard now keeps it in a ThreadLocal<object> (framework-owned storage), and Current() reads IsValueCreated first.
  • Evidence: before, 1 crash in 40 full net10 runs on this matrix leg (and 2 on main); after, 0 in 120 (p ≈ 5% to see that by chance at the old rate).

2. GuardedCodeKeepsItsMeaning (Monitor row) timed out after 2 min on net8

  • A real bug: a sliced Monitor.Wait leaves the wait queue when a slice times out, until it holds the lock again; a Pulse in that gap was lost and the waiter slept until the budget.
  • Fix: the agent's Monitor.Pulse / PulseAll become guard stand-ins that count the wake-ups owed to the object's sliced waiters; only that count decides who wakes, so a pulse is never lost and one Pulse still wakes one waiter. Contract §3.6 + changelog updated (pulses from code the guards do not rewrite can still be missed).
  • New deterministic rows (the pulsing thread holds the lock longer than a slice): fail on main, pass here.

Quality stages: code review (findings fixed in e2ce285: no double wake-up) and simplify (1ceed14). Local: 332/332 on net10 (Roslyn 3.8).

🤖 Generated with Claude Code

IvanMurzak and others added 4 commits November 6, 2026 02:08
…s is not lost

A slice that times out leaves the wait queue until its thread holds the lock
again; a Pulse sent in that gap was lost, and the waiter slept until the
budget (GuardedCodeKeepsItsMeaning failed intermittently in CI). The agent's
Monitor.Pulse / PulseAll now become guard stand-ins that count what they owe
the object's sliced waiters; a waiter between two slices takes it as its
wake-up. New deterministic test: the pulsing thread holds the lock longer
than a slice.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…pet assembly

.NET 10 test runs crashed now and then in the GC: a segfault in
CFinalize::ScanForFinalization on an object with a null method table
(crash dump from CI, Roslyn 4.0.1 line, symbolized). The guard's run
tracking used a [ThreadStatic] of a snippet type plus generic instantiations
over snippet types (AsyncLocal<Run>, ConditionalWeakTable<object, Waiting>)
inside the collectible assembly; dead threads keep thread-static storage of
an unloaded context. They are now ThreadLocal<object> / AsyncLocal<object> /
ConditionalWeakTable<object, object>, framework instantiations that are not
collectible. Contract changelog notes the Monitor fix.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The owed-wake-up count is now the only signal: while sliced waiters exist a
Pulse becomes PulseAll plus one owed wake-up, the first waiter to hold the
lock takes it and the others wait on. Before, the native pulse and the count
could each wake a different waiter. Contract notes the limits (waits and
pulses outside the guarded code). New test: two Pulses, three waiters.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…simplify)

Plain Pulse again (only the owed count decides who wakes, so no PulseAll and
no extra wake-ups), one ElapsedMs helper for Sliced and TakeWakeUp, one
WaitingOn lookup; AsyncLocal<Run> and the typed table back (only the
thread-local needed to move off collectible thread-statics); Current() reads
IsValueCreated first so a read allocates nothing; the comment states the
crash cause as the suspect it is. Contract: the Pulse note is a sentence of
its own. Monitor tests folded into GuardedCodeKeepsItsMeaning rows.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@IvanMurzak
IvanMurzak merged commit 76ee4da into main Nov 6, 2026
13 checks passed
@IvanMurzak IvanMurzak mentioned this pull request Nov 6, 2026
IvanMurzak added a commit that referenced this pull request Nov 6, 2026
* chore: release 0.4.1 (GC crash fix, Monitor.Pulse in sliced waits)

Version 0.4.0 -> 0.4.1 in dotnet/Directory.Build.props and the README snippet; the contract's
Unreleased changelog entry becomes v0.4.1 and also names the ThreadLocal change from #14.
The Python module is unchanged since 0.4.0 and keeps its version.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* simplify: no changes

Version bump and changelog only; no code to reuse, simplify or speed up.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* code review: no findings

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
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.

1 participant