fix(cmux-next pages): a failed subscribe releases its early-event list on every exit

Merged
LA
7d ago
+106-0

Summary

A page whose provider's subscribe fails (it throws, or the page closes while it runs) leaked memory for the rest of the session when that provider kept its onEvent closure: every event it still sent was appended to a list nobody reads.

PageRouter.subscribe makes a PageEarlyEvents list that the provider's onEvent captures, to hold events sent before the subscribe reply. It released the list only on the success paths. On the throw and closed exits it stayed open, so later events kept growing it. Now one owner frees it on every exit: a defer releases it unless the posted flush has taken it over. After the release, a late event of a failed subscribe goes to deliver, which drops it (the stream is not live).

Found in the review of #27468 (same function; the two merge cleanly). The same leak is fixed on perf/chrome-parity in cp/fix-tip-page-router-release-on-throw.

Changelog

Fixed: A page whose stream failed to start no longer keeps memory for every event its provider still sends.

Testing

Regression test PageRouterSubscribeEndTests (red commit fee857bc4c1 first, fix b3a4dffc4ac, test shape ea37a897254). A provider keeps onEvent and its subscribe throws, or closes the page; 10 000 events it sends afterwards must add 0 main-thread allocations over the same empty loop (counted through libmalloc's malloc_logger; a self-check fails the test when the hook sees nothing) and reach no page. The empty loop is subtracted because a Debug for-in allocates once per iteration on the fleet (probe 5529e509: an empty loop and every step of the drop path each counted exactly 10 000).

cmux-ci run --class light --script scripts/ci/package-test-lane.sh --arg suite --arg Packages/macOS/CmuxNext --arg PageRouterSubscribeEndTests (through proof-step.py):

Compile: cmux-ci compile-check --app --package CmuxNext --recipe-version 2 at ea37a897254 (fleet 6dc98323567433da9601e981): passed.

Observed / Not run

Observed: the fleet runs above; scripts/cmux-next/prepush-gate.py clean.
Not run: a Release run on feat itself (feat's Release test build does not compile yet: UiLayerPreviewTests and two allocation tests; fixed on perf in cp/fix-tip-release-test-compile).

Proof

No capture: memory-only fix in the page router; nothing on screen changes.

Review (independent subagent, correctness first): SHIP at ea37a89725480824a31af321a2423b2bed088687

Follow-ups noted in review, not in this PR: a test comment names CrashKeySlotsTests, which is on perf only; after #27468 the nothing-held path releases the list twice (harmless, release() is idempotent); and an older issue: a reset() while a provider's subscribe is suspended registers the old subscription in the new document.

Checklist

  • Behavior changes have added or updated tests (PageRouterSubscribeEndTests)
  • Reviewed with a subagent before merge; verdict above. No user-facing strings change.

View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need.

Summary by cubic

Fixes a memory leak in PageRouter.subscribe where a failed subscription kept its early-event list alive for the rest of the session.

When a provider's subscribe threw, or the page closed while it ran, the PageEarlyEvents list captured by onEvent was never released. A provider that kept onEvent would append every later event to a list nobody reads. A defer now releases the list on every exit unless the posted flush has taken ownership, so late events of a failed subscribe are dropped immediately.

Bug Fixes

  • Failed subscriptions now release their early-event list on all exit paths.
  • Added PageRouterSubscribeEndTests to verify a failed subscribe allocates nothing for 10 000 events sent afterwards.