Skip to content

qmux: deliver byte events on stream write, with tests + CMake wiring#625

Open
afrind wants to merge 1 commit into
facebook:mainfrom
afrind:qmux-byte-event-delivery-fix
Open

qmux: deliver byte events on stream write, with tests + CMake wiring#625
afrind wants to merge 1 commit into
facebook:mainfrom
afrind:qmux-byte-event-delivery-fix

Conversation

@afrind

@afrind afrind commented Jun 20, 2026

Copy link
Copy Markdown
Contributor

QmuxSession::writeLoop dropped the dequeued delivery callback, so the app's ByteEventCallback registration (and any keepalive it held) leaked and onByteEvent never fired. Fire deliveryCallback->onByteEvent after the buffered write is dequeued and written to the wire.

Add QmuxSessionTest coverage:

  • WriteWithByteEventCallback_FiresOnByteEvent: the fix itself.
  • CloseSession_CancelsQueuedByteEvent: session close cancels a still queued byte event (shutdown -> WriteHandle::cancel -> clear -> onByteEventCanceled) rather than leaking it.

Wire the qmux tests into CMake (they were buck-only):

  • new proxygen/lib/transport/qmux/test/CMakeLists.txt registering all four qmux tests against granular library targets (not the monolithic proxygen target).
  • new proxygen/lib/http/coro/transport/test/CMakeLists.txt building the TestCoroTransport test util against granular targets.
  • add_subdirectory(test) hooks in the qmux and coro/transport parents.

QmuxSession::writeLoop dropped the dequeued delivery callback, so the
app's ByteEventCallback registration (and any keepalive it held) leaked
and onByteEvent never fired. Fire deliveryCallback->onByteEvent after the
buffered write is dequeued and written to the wire.

Add QmuxSessionTest coverage:
- WriteWithByteEventCallback_FiresOnByteEvent: the fix itself.
- CloseSession_CancelsQueuedByteEvent: session close cancels a still
  queued byte event (shutdown -> WriteHandle::cancel -> clear ->
  onByteEventCanceled) rather than leaking it.

Wire the qmux tests into CMake (they were buck-only):
- new proxygen/lib/transport/qmux/test/CMakeLists.txt registering all
  four qmux tests against granular library targets (not the monolithic
  proxygen target).
- new proxygen/lib/http/coro/transport/test/CMakeLists.txt building the
  TestCoroTransport test util against granular targets.
- add_subdirectory(test) hooks in the qmux and coro/transport parents.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@meta-cla meta-cla Bot added the CLA Signed label Jun 20, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant