Skip to content

test(ffi): hold the route that keeps the panicking origin alive - #7989

Merged
pullfrog[bot] merged 1 commit into
quest/m1/api-audit/route/handlefrom
pullfrog/7806-supervision-test-route
Nov 5, 2026
Merged

pullfrog[bot] merged 1 commit into
quest/m1/api-audit/route/handlefrom
pullfrog/7806-supervision-test-route

Conversation

@pullfrog

@pullfrog pullfrog Bot commented Nov 5, 2026

Copy link
Copy Markdown
Contributor

Fix for the moq-ffi check failure on #7806, stacked on its branch because the branch is locked in the merge queue.

Diagnosis

origin::supervision_tests::container_importers_keep_the_origin_failure_after_other_owners_are_released failed on CI 114130473307 with:

assertion failed: matches!(completion.finished().await, Err(MoqError::Task { .. }))

The test was migrated to origin.route(...).unwrap().broadcast().unwrap(), which drops the MoqRoute and keeps only broadcast handles. Under the new ownership the route is the origin's owner (RouteState::_keepalive), while a broadcast producer is not: once the origin producer and the route are both gone, the driver polls Closed and finishes cleanly. The test then races the driver against its injected panic, so it usually passes on an idle host and loses the race under CI load.

Verified deterministically by dropping the route before the panic and waiting 200 ms: the assertion fails with the same message. Holding the route instead passes 15/15 runs.

Change

  • Hold the route for the duration of the failure check, then drop it before the importer assertions, so only the importers still carry the origin's failure. This keeps the test's intent instead of weakening it.
  • Clarify the Route doc: the route, not the broadcast, keeps the origin's Driver running.

Verification

  • cargo test -p moq-ffi --lib -- origin::supervision_tests: 7 passed.
  • cargo fmt --all -- --check and cargo clippy -p moq-ffi --lib --locked -- -D warnings: clean.

Once #7806 is dequeued, this commit (da3f1b09e) can go straight onto quest/m1/api-audit/route/handle.

Pullfrog  | View workflow run | via Pullfrog | Using deepseek-v4.1-flash-1017 (free via Pullfrog for OSS) | 𝕏

The container-importer supervision test dropped the route it created as a
temporary, leaving only broadcast handles. Under the new ownership the
route is the origin's owner, so the driver could observe every owner gone
and finish cleanly before the injected panic, making the test race the
driver and fail under load. Hold the route until the failure is recorded,
then drop it before the importer assertions.

Also spell out that the route, not the broadcast, keeps the driver
running.
@pullfrog
pullfrog Bot requested a review from kixelated November 5, 2026 22:48
@pullfrog
pullfrog Bot merged commit da3f1b0 into quest/m1/api-audit/route/handle Nov 5, 2026
3 checks passed
@pullfrog
pullfrog Bot deleted the pullfrog/7806-supervision-test-route branch November 5, 2026 22:49
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.

0 participants