Fix/perf client segfault - #217
Closed
peterchave wants to merge 469 commits into
Closed
Conversation
The hardcoded limit of 100 simultaneous QUIC connections was a leftover default that blocks production deployments with more concurrent clients. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Adds point-in-time samples (smoothedRttUs, receiveBytesPerSec, bytesInTransit) to PathQualityDelta and enables path quality change callbacks via picoquic_default_quality_update so callers receive the event when pacing rate or RTT shifts meaningfully. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
sync: upstream moxygen 5e46376
sync: openmoq/picoquic pin to 8c14cc449afc
sync: openmoq/picoquic pin to 0c516acd3b57
sync: openmoq/picoquic pin to ee6f0b1fb491
Add onPacketsSent(n) and onPacketsReceived(n) to PicoQuicStatsCallback with default no-op implementations. Bump them from PicoQuicSocketHandler: - drainOutgoing() calls onPacketsSent with total packets sent per drain pass - onNotifyDataAvailable() calls onPacketsReceived with recvmmsg batch total Wire the callback into PicoQuicSocketHandler via setStatsCallback(), called from MoQPicoQuicEventBaseServer::start() using a new protected statsCallbackRaw() accessor on MoQPicoServerBase. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
sync: openmoq/picoquic pin to 6e8a5261db61
sync: openmoq/picoquic pin to 5e4c01dd72cd
sync: openmoq/picoquic pin to 83a8e0d5f12b
…TER tests Two MoQFramerV16PlusTest cases (SubscribeNamespaceWithTrackFilter, TrackFilterLargeValues) asserted SUBSCRIBE_NAMESPACE wire type 0x50. That value is draft 18+ per draft-ietf-moq-transport-18 section 10.18; draft 16 (the only version this suite is instantiated for) uses 0x11 (LEGACY_SUBSCRIBE_NAMESPACE) per draft-ietf-moq-transport-16 section 9.25. MoQFrameWriter::writeSubscribeNamespace already branches on majorVersion >= 18 to emit the correct wire type, so the writer was spec-compliant; the test expectation was stale relative to the v18 wire-type renumber introduced upstream by b07c680. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
sync: upstream moxygen 56ea808
sync: upstream moxygen 5031779
sync: openmoq/picoquic pin to 8e9d619d9014
sync: upstream moxygen 8ef5bd1
Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
sync: upstream moxygen 1ef24a9
sync: openmoq/picoquic pin to cb82e2aaf3e1
sync: upstream moxygen d1dd07e
sync: upstream moxygen 05a1df1
sync: upstream moxygen 5da613e
sync: upstream moxygen 1bd355e
sync: openmoq/picoquic pin to 02b5452b6ec7
Summary: fMP4 parse into a per-track Fmp4Track; the static single-object CatalogSource (catalog served as a track); the prefix-routed FileMediaSourceResolver. Reviewed By: kvtsoy Differential Revision: D113852924 fbshipit-source-id: 130da6de1a870edefc2a036930a7360447ec2fd4
Summary: Drain a SegmentSource into a MoQForwarder - subgroup/stream mapping, BLOCKED backpressure, cancellable teardown. Reviewed By: kvtsoy Differential Revision: D113852925 fbshipit-source-id: b57541ae16df3469b28a4140a2695cee1b32afdd
Summary: GitHub commits: facebook/CacheLib@0c10a77 facebook/fb303@aa25fe8 facebook/fbthrift@276ca28 facebook/folly@4963d7c facebook/hermes@a7086fc facebook/mvfst@d532bc0 facebook/proxygen@7ef66a5 facebook/pyrefly@941711b facebook/wangle@db6cfcf facebookexperimental/rust-shed@3ca7aef facebookincubator/fizz@4c9c9b9 react/yoga@909c1b8 Reviewed By: vladi99 fbshipit-source-id: 13ce2244acbc6bd54afd9c7bad2a18e5316e7cc9
Summary: # Context Endpoint resolution lived in `MoQClientBase` before transport selection, forcing every client path through `folly::SocketAddress` and preventing mobile QUIC from supplying asynchronous DNS. # This Diff Moves endpoint resolution into the QUIC-specific connect implementations. `MoQClientBase` retains URL and deadline orchestration, while each QUIC client owns the address type and resolution strategy it needs. TCP/QMUX behavior is unchanged. # This Stack 1. [NFC] Avoid copies in client test callbacks. 2. **Move endpoint resolution behind QUIC client implementations.** 3. Add the singular async QUIC address seam and deadline accounting. 4. Adapt MNS DNS to the QUIC endpoint contract. 5. Test the MNS QUIC DNS adapter. 6. Inject MNS DNS into TigonMoQ on Android. 7. Add detached event-loop shutdown. 8. Avoid blocking Android runtime destruction during system DNS. Reviewed By: kvtsoy Differential Revision: D114627471 fbshipit-source-id: cba4130b8052eca7d01e1e72a3e5b49197aea637
Summary: # Context Synchronous URL resolution blocks the mobile moxygen event loop and cannot participate in coroutine cancellation or the connection deadline supplied by the caller. The resolver seam must remain source-compatible until production clients inject a resolver later in the stack. # This Diff Adds the singular `MoQQuicAddressResolver` coroutine seam returning the owning `quic::SocketAddress` used by mvfst on each platform. When a resolver is supplied, `MoQClientMobile` applies a caller-side timeout that cancels the returned task, preserves structured cancellation, and charges DNS plus QUIC setup to the existing end-to-end deadline. Omitting the resolver preserves the existing synchronous fallback so this commit and standalone callers remain independently buildable. The mobile text client resolves during CLI startup, before running its event loop, and injects that pre-resolved endpoint explicitly. Because mvfst receives a bare IP, `setHostname(url_.getHost())` preserves the original host for TLS/SNI and matches the desktop path. `QuicConnectCB::onTransportReady` also returns after reporting cancellation, fixing a pre-existing second completion of the promise. # This Stack 1. [NFC] Avoid copies in client test callbacks. 2. Move endpoint resolution behind QUIC client implementations. 3. **Add the singular async QUIC address seam and deadline accounting.** 4. Adapt MNS DNS to the QUIC endpoint contract. 5. Test the MNS QUIC DNS adapter. 6. Inject MNS DNS into TigonMoQ on Android and remove the fallback. 7. Add detached event-loop shutdown. 8. Avoid blocking Android runtime destruction during system DNS. Reviewed By: JunqiWang Differential Revision: D114613259 fbshipit-source-id: c7c0e96886fdc6b0767647e75678d47c67bade8b
Summary: # Context The preceding async resolver seam remains optional for source compatibility. Android must create an MNS resolver with the correct event-loop lifetime, inject it into TigonMoQ, and prevent production clients from silently falling back to blocking DNS. # This Diff Makes `MoQClientMobile` and `TigonMoQClient` require a non-null address resolver and removes the synchronous fallback. The Android runtime creates an MNS DNS resolver on its event loop, wraps and injects it through the QUIC resolver seam, and drains it during loop-owned shutdown. Client tests cover DNS errors and distinguish caller cancellation as `folly::OperationCancelled` from deadline expiry as `folly::FutureTimeout`; malformed MNS output is covered by the adapter tests earlier in the stack. The client target uses Folly shared test main because these timeout paths require `folly::Init` to register the Timekeeper singleton. # This Stack 1. [NFC] Avoid copies in client test callbacks. 2. Move endpoint resolution behind QUIC client implementations. 3. Add the singular async QUIC address seam and deadline accounting. 4. Adapt MNS DNS to the QUIC endpoint contract. 5. Test the MNS QUIC DNS adapter. 6. **Inject MNS DNS into TigonMoQ on Android and remove the fallback.** 7. Add detached event-loop shutdown. 8. Avoid blocking Android runtime destruction during system DNS. Reviewed By: JunqiWang Differential Revision: D114613258 fbshipit-source-id: dc8dedce7209de6bb8a96866c54f95c84fcafcc1
Summary: Make `MoQExecutor` a typealias for `quic::QuicExecutor` ___ Differential Revision: D114163978 fbshipit-source-id: 6ca671a8dbf7d9d1c9548729a65244056b01743e
Upstream changed moxygen::MoQExecutor from a class to `using MoQExecutor = quic::QuicExecutor`, so the forward declaration in MoQPicoServerBase.h now conflicts with the alias. Include moxygen/events/MoQExecutor.h instead of forward declaring it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
sync: upstream moxygen 4005310
- install-system-deps.sh: dispatch on ID_LIKE when ID is not a known distro, so Ubuntu/Debian derivatives (Linux Mint, Pop!_OS, LMDE) and RHEL derivatives (Rocky, Alma) install deps instead of erroring out. - omoq-ci-main.yml: add ubuntu-24.04-amd64 to the publish matrix so the snapshot release covers the current Ubuntu LTS; the release job picks it up automatically (downloads all artifacts). - omoq-dev-build.yml: add ubuntu-24.04-amd64 target for parity with the publish matrix.
Consumers of the installed tree (e.g. moqx) locate GoogleTest with find_package(GTest REQUIRED CONFIG). The system-googletest probe used module mode, so a googletest visible only to FindGTest.cmake (libs and headers without a cmake package config) short-circuited the FetchContent bundling and produced an install tree those consumers cannot configure against. Require CONFIG in the probe so bundling is skipped only when a config-mode googletest is genuinely available.
sync: upstream moxygen 2308605
meta-cla
Bot
added
the
CLA Signed
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
6 participants
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.
Here is the proposed fix for the perf client crash for review.