Skip to content

fix(scheduler): matching engine no longer exits when polled during construction - #41

Open
b7r6 wants to merge 2 commits into
mainfrom
fix/matching-engine-startup
Open

b7r6 wants to merge 2 commits into
mainfrom
fix/matching-engine-startup

Conversation

@b7r6

@b7r6 b7r6 commented Oct 7, 2026 •

Copy link
Copy Markdown
Owner

What and why

Lane A stability fix from the findings catalog (see nativelink-findings-2026-10-05.csv
on the meta/findings-catalog-2026-10-05 branch), rebased onto OSS main
@bf4b3aa855. This fork PR exists to validate the change on CI that is
representative of upstream PR lanes (public read-only cache, same endpoints)
before any upstream submission.

How was this verified?

The branch carries its own regression tests (fails-without where the fix
class allows it — see the branch's test commits). This PR's purpose is the
full representative CI run; green here rehearses the exact checks an
upstream PR would face.

Risk

Fork-internal validation PR; nothing lands anywhere by merging this. The
underlying fix's risk assessment happens at upstream-submission time.

AI assistance

Agent-assisted pipeline (Claude) for rebase and PR mechanics under my
direction; the fix itself carries verification in its tests.

@github-actions

This comment has been minimized.

@b7r6
b7r6 force-pushed the fix/matching-engine-startup branch 3 times, most recently from 78905ca to 373d2ba Compare October 8, 2026 17:51
@b7r6
b7r6 force-pushed the fix/matching-engine-startup branch 3 times, most recently from f65a385 to 031903f Compare October 15, 2026 13:51
b7r6 and others added 2 commits October 21, 2026 17:14
Completes the no-per-PR-OCI-builds intent: image-off-pr moved image.yaml off
PRs and TraceMachina#2877 moved the nix integration job off PRs, but main.yaml's
integration-tests (docker-compose image build + run_integration_tests.sh)
fell between them and still ran on every PR. Defer to nightly + merge-to-main,
matching TraceMachina#2880's pattern. Fork-local until folded into the upstream OCI/nightly
set.
…nstruction

The matching-engine task is spawned inside `Arc::new_cyclic`, during
which `weak_inner.upgrade()` returns `None` because the strong count is
still zero. The task treats a failed upgrade as scheduler shutdown, so
if `task_change_notify` already held a permit (e.g. a Redis pubsub
event arriving before `SimpleScheduler::new`), its first poll completed
instantly, consumed the permit, saw `None`, and returned permanently —
a scheduler that never matches anything.

Gate the task's loop on a oneshot fired unconditionally right after
`Arc::new_cyclic` returns, so the task can neither consume a pre-stored
notify permit nor observe the partially constructed scheduler.

Includes a deterministic in-crate repro (adversarial review fleet,
branch repro/me-constructor-race, d44e3cb) that pre-stores a notify
permit and widens the in-closure window via a test-only timing seam;
it fails without the fix and passes with it.

Co-Authored-By: Claude Fable 5 <[email protected]>
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