Skip to content

fix(libmoq): update tests for moq_origin_publish handle return#1710

Merged
kixelated merged 1 commit into
devfrom
fix/libmoq-stale-publish-handle-tests
Jun 13, 2026
Merged

fix(libmoq): update tests for moq_origin_publish handle return#1710
kixelated merged 1 commit into
devfrom
fix/libmoq-stale-publish-handle-tests

Conversation

@kixelated

@kixelated kixelated commented Jun 13, 2026

Copy link
Copy Markdown
Collaborator

Summary

cargo test -p libmoq --all-features was crashing with a SIGSEGV (signal 11) and a non-unwinding panic. The failing tests (local_announce, catalog_update_on_new_track, announced_deactivation, consume_announced_local, double_close_all_resource_types, and others) all panicked first on an assertion like assert_eq!(moq_origin_publish(...), 0).

Root cause: stale tests, not a callback bug. #1640 changed moq_origin_publish to return a positive publish handle (paired with the new moq_origin_unpublish) under the RAII OriginPublish model, but never updated rs/libmoq/src/test.rs, which still asserted a 0 return. This went unnoticed because CI's cargo check --workspace --all-features failed to compile (NVENC) since #1691, so cargo test --all-features never actually ran until the moq-video CI plumbing was fixed in #1704.

Why it segfaulted. The failed == 0 assertion panicked each test mid-flight, which dropped the test's Callback (freeing user_data) without draining the terminal callback via recv_terminal(). The still-alive background announce/catalog task then fired its callback into freed memory: a use-after-free → SIGSEGV. The terminal-callback contract in the production code (free user_data exactly once, in the final callback) was already correct (#1546) and is unchanged.

Changes (test-only)

  • Capture the publish handle with id() (which still asserts it is positive) at all 10 call sites, instead of asserting == 0.
  • announced_deactivation previously deactivated by closing the broadcast producer. Under the OriginPublish model, dropping the producer no longer unannounces, so it now deactivates via moq_origin_unpublish(publish) and closes the broadcast at the end.

Test plan

  • cargo test -p libmoq --all-features — 23 passed, 0 failed, no abort (run 3×, repeatable)
  • cargo test -p libmoq (default features) — 23 passed
  • cargo fmt -p libmoq --check — clean
  • cargo clippy -p libmoq --all-features — clean

No public API or wire behavior changes (test-only). Targets dev, since the moq_origin_publish handle change (#1640) and the CI fix that surfaced this (#1704) both live there.

(Written by Claude)

`moq_origin_publish` was changed in #1640 to return a positive publish
handle (paired with `moq_origin_unpublish`) under the RAII OriginPublish
model, but the libmoq tests still asserted it returned 0. The staleness
went unnoticed because CI's `cargo check --all-features` failed to compile
(NVENC) since #1691, so these tests never ran.

The failed `== 0` assertion panicked each test mid-flight, dropping the
test's Callback (freeing user_data) without draining the terminal callback,
so the still-alive background task fired into freed memory: a SIGSEGV under
the test harness. The callback contract itself (free user_data once, in the
final terminal callback) was already correct.

Capture the handle with `id()` (which still checks it is positive) at every
call site. `announced_deactivation` deactivated by closing the broadcast,
but dropping the producer no longer unannounces under the OriginPublish
model, so it now deactivates via `moq_origin_unpublish` instead.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@kixelated
kixelated changed the base branch from main to dev June 13, 2026 05:48
@kixelated
kixelated enabled auto-merge (squash) June 13, 2026 05:49
@kixelated
kixelated disabled auto-merge June 13, 2026 14:34
@kixelated
kixelated merged commit dad9aad into dev Jun 13, 2026
@kixelated
kixelated deleted the fix/libmoq-stale-publish-handle-tests branch June 13, 2026 14:35
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