MoQForwarder: restore passive-subscriber guard in duplicate beginSubgroup#326
Merged
Conversation
…roup The sync merge of upstream e848acf ("Gate subgroup reopen on v18 forward resume") rewrote the duplicate-subgroup block in MoQForwarder::beginSubgroup and dropped the openmoq guard added in 12b24a6: a passive subscriber (the relay's top-N/cache observer chain) whose stale consumer gets reset must not count toward anyReset, otherwise CANCELLED never propagates to the publisher once all real consumers have stop_sent. Restore that guard and also exclude passive subscribers from the new anyReopenCandidate check - a passive sub with no subgroup entry would mask CANCELLED the same way. Add two regression tests so moxygen CI catches the next sync that drops the guard, instead of it surfacing two repos downstream as the moqx DuplicateSubgroupCancelledWhenNoActiveConsumers/LocalForwarderMT segfault (moqx #499).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
moqx sync PR openmoq/moqx#499 fails on all 3 platforms:
DuplicateSubgroupCancelledWhenNoActiveConsumers/LocalForwarderMTsegfaults (ASAN teardown use-after-free).Root cause: the sync-conflict resolution in #324 (pin
0d9f3af) took upstreame848acf5("Gate subgroup reopen on v18 forward resume") verbatim for the duplicate-subgroup block inMoQForwarder::beginSubgroup, dropping the openmoq guard from12b24a60: when a duplicate subgroup resets a passive subscriber's stale consumer (the relay's top-N/cache observer chain), it must not count towardanyReset. Without the guard, the passive local-forwarder subscriber masks the "no active consumers" signal, the duplicatebeginSubgroupsucceeds instead of returning CANCELLED, and the moqx test'sdupRes.error()on a success value throws, skipping cleanup and segfaulting in teardown.Same failure pattern as the recurring TRACK_FILTER
kParamAllowlistsync casualty.Fix
MoQForwarder::beginSubgroup: restore the!sub->passiveguard onanyReset, and exclude passive subscribers from upstream's newanyReopenCandidateas well (a passive sub with no subgroup entry would otherwise mask CANCELLED the same way). Reconciles our passive-subscriber feature with upstream's v18 reopen logic rather than reverting it.OpenMOQForwarderTestso moxygen's own CI catches the next sync that drops this guard, instead of it surfacing two repos downstream in moqx.Validation
OpenMOQForwarderTest: 13/13 pass with the fix; both new tests fail against the unguarded post-sync code (verified by stashing the fix).MoQForwarderTest: 26/26 pass — the v18 forward-resume reopen behavior is unchanged for real subscribers.Follow-up
On merge, snapshot publish → bump the moxygen pin on the existing
sync-moxygen/*branch in moqx (keeping the compile fix already there) so #499 goes green and auto-merges.This change is