Skip to content

feat: add mux.cool outbound - #2980

Open
Jolymmiles wants to merge 10 commits into
MetaCubeX:Alphafrom
Jolymmiles:Alpha
Open

feat: add mux.cool outbound#2980
Jolymmiles wants to merge 10 commits into
MetaCubeX:Alphafrom
Jolymmiles:Alpha

Conversation

@Jolymmiles

@Jolymmiles Jolymmiles commented Jul 16, 2026

Copy link
Copy Markdown

Summary

This PR adds native Xray-compatible mux.cool client and server support for TCP, UDP, and XUDP traffic.

  • Implements the Xray Mux wire codec for New, Keep, End, and KeepAlive frames.
  • Supports multiplexed TCP streams and UDP/XUDP packet sessions.
  • Preserves UDP datagram boundaries and destination addresses.
  • Implements XUDP Global ID handling and per-datagram destination metadata.
  • Adds full-duplex logical sessions with deadline support and per-stream TCP context cancellation.
  • Implements carrier workers and connection pooling.
  • Establishes carriers through the base proxy to v1.mux.cool:9527.
  • Encodes v1.mux.cool:9527 carriers with the native VLESS Mux command, matching Xray-core in both client/server directions.
  • Wraps compatible outbound proxy types generically, including Trojan, VMess, and VLESS.
  • Accepts v1.mux.cool carriers through the shared sing-based listener path, including Trojan, VMess, and VLESS listeners.
  • Supports a shared primary pool or an optional dedicated UDP/XUDP packet pool.
  • Supports active-session and lifetime-session limits per carrier.
  • Adds a strict shared limit for physical TCP carriers across the primary and dedicated packet pools.
  • Handles carrier reuse, rotation, idle cleanup, failure propagation, and idempotent session termination.
  • Adds Xray-compatible UDP/443 handling with reject, allow, and skip policies.
  • Rejects configurations that enable both smux and mux.cool.
  • Does not silently fall back to the underlying proxy when Mux fails.
  • Allows bypassing Mux for UDP/443 only when explicitly configured with skip.
  • Optimizes frame encoding, decoding, session lifecycle, carrier writes, and UDP receive paths.
  • Adds real mihomo/Xray cross-runtime process tests for TCP, UDP, and XUDP in both directions.

Configuration

mux.cool:
  enabled: true
  max-concurrency: 8
  max-connections: 128
  max-carriers: 4
  xudp-concurrency: 4
  xudp-proxy-udp443: reject

Parameters

enabled

Enables mux.cool for the proxy.

When enabled, logical TCP, UDP, and XUDP sessions can be transported over multiplexed TCP carriers connected through the underlying proxy to v1.mux.cool:9527.

Default: false.

max-concurrency

Maximum number of simultaneously active logical sessions on one primary Mux carrier.

When every existing carrier reaches this limit, another physical carrier is opened. If xudp-concurrency is zero, this limit applies to TCP, UDP, and XUDP sessions in the shared pool.

Default: 8.

A zero or omitted value uses the default.

max-connections

Maximum total number of logical sessions that one carrier may accept during its lifetime.

After a carrier has accepted this many logical sessions, it stops accepting new sessions. Existing sessions continue until completion, while new sessions are assigned to another carrier.

Despite the option name, this is not a limit on the number of physical carrier connections. It is a lifetime logical-session limit for each carrier.

The limit applies independently to carriers in the primary and dedicated packet pools.

Default: 128.

A zero or omitted value uses the default.

max-carriers

Strict maximum number of physical TCP carrier connections owned by this mux.cool wrapper.

The limit is shared by the primary pool and the optional dedicated UDP/XUDP packet pool. In-flight carrier dials count toward the limit, preventing concurrent dial attempts from overshooting it.

When the limit is reached, new logical sessions wait for capacity on an existing carrier or for a physical carrier slot to be released. Waiting respects caller context cancellation and pool shutdown. Mux is not bypassed and the limit is never silently exceeded.

Carrier capacity is released after dial failure, carrier rotation, idle cleanup, carrier failure, or pool shutdown.

Default: 0 (unlimited, preserving the previous behavior).

A negative value is rejected during configuration parsing.

xudp-concurrency

Controls packet-pool isolation and the maximum number of active UDP/XUDP sessions per packet carrier.

  • 0: TCP, UDP, and XUDP use the same primary carrier pool.
  • Greater than 0: creates a dedicated UDP/XUDP packet pool and uses the configured value as its per-carrier active-session limit.
  • Less than 0: rejected as invalid configuration.

A dedicated packet pool prevents long-lived UDP/XUDP sessions from consuming per-carrier TCP session capacity. The implementation routes all packet sessions through this pool when it is enabled, including sessions without an XUDP Global ID. When max-carriers is non-zero, both pools still share the same physical connection budget.

Default: 0.

xudp-proxy-udp443

Controls how UDP traffic targeting port 443, typically QUIC or HTTP/3, is handled.

Supported values:

  • reject: reject UDP/443 before opening a carrier. This is the default and matches Xray-core behavior.
  • allow: transport UDP/443 through the mux packet path.
  • skip: bypass mux.cool and delegate UDP/443 directly to the underlying proxy.

The policy applies only to UDP/443. Other UDP destinations continue through the shared or dedicated packet pool.

Default: reject.

Unknown values are rejected during configuration parsing.

Example Behavior

With the configuration shown above:

  • Each primary TCP carrier can have up to 8 active logical sessions.
  • UDP/XUDP uses a separate packet carrier pool.
  • Each packet carrier can have up to 4 active packet sessions.
  • The primary and packet pools can own at most 4 physical TCP carriers in total.
  • Every carrier is rotated after accepting 128 logical sessions over its lifetime.
  • UDP/443 is rejected.
  • Other UDP traffic is transported through the dedicated packet pool.

Protocol Support

The implementation supports:

  • Native client and server operation between mihomo instances.
  • Bidirectional VLESS interoperability between mihomo and Xray-core clients and servers.
  • Generic client-side wrapping of compatible base proxies, including Trojan, VMess, and VLESS.
  • Native carrier handling through sing-based server listeners, including Trojan, VMess, and VLESS.
  • TCP logical streams.
  • UDP packet sessions with preserved datagram boundaries.
  • XUDP packet sessions using an 8-byte Global ID.
  • Per-datagram target metadata in XUDP Keep frames.
  • IPv4, IPv6, and domain destinations.
  • Server-first TCP protocols.
  • Shared TCP and packet-session carrier reuse.
  • Optional dedicated UDP/XUDP packet carrier pools.
  • Cross-carrier XUDP rebinding while preserving the Global ID.
  • Concurrent stream and packet sessions on the same carrier.

TCP, UDP, and XUDP failures are propagated to the caller without silently bypassing Mux.

The only bypass behavior is the explicit xudp-proxy-udp443: skip policy.

Connection Management

Each carrier tracks:

  • Currently active logical sessions.
  • Total logical sessions created during its lifetime.
  • Session identifiers and routing state.
  • Idle state and cleanup timers.
  • Read and write failures.

The pool supports:

  • Carrier reuse while capacity is available.
  • New carrier creation when the concurrency limit is reached.
  • Strict global physical-carrier limiting when max-carriers is configured.
  • Context-aware waiting when all physical carrier slots are occupied.
  • Waking all blocked callers when reusable logical capacity becomes available.
  • Carrier rotation when the lifetime-session limit is reached.
  • Idle carrier cleanup.
  • Context-aware carrier creation.
  • Failure propagation to affected logical sessions.
  • Idempotent shutdown.
  • Closing the primary and dedicated packet pools before the underlying proxy.

When a dedicated packet pool is configured, it has independent workers, concurrency accounting, carrier rotation, and idle cleanup. Its physical carriers still consume the same max-carriers budget as the primary pool.

Error Handling

  • Carrier creation errors are returned to the caller.
  • Failed carrier dials release their reserved physical-carrier slots.
  • Waiting for physical-carrier capacity respects context cancellation and pool shutdown.
  • Mux failures do not silently create regular TCP or UDP connections.
  • Malformed frames terminate the affected carrier.
  • Carrier EOF and write failures are propagated to logical sessions.
  • TCP dial-context cancellation closes only the associated logical stream.
  • Packet sessions intentionally outlive the context used to create them; their lifetime is controlled through the returned PacketConn, deadlines, carrier failure, or pool shutdown.
  • Closing a logical session sends End at most once.
  • UDP/443 rejection occurs before carrier creation.
  • Explicit skip mode returns the underlying proxy result directly.

Performance

The hot paths were optimized through a bounded 20-pass benchmark- and profile-driven performance cycle.

Each pass started from a benchmark, CPU profile, allocation profile, contention profile, or process-level observation. Only changes with measurable improvements and preserved protocol semantics were retained.

Implemented optimizations:

  • Reuses carrier-owned frame encoding buffers.
  • Reuses worker-owned metadata decoding buffers.
  • Reuses the metadata scratch area for frame-length decoding.
  • Uses pooled ownership for internally decoded payload buffers.
  • Returns pooled payloads only after TCP or UDP consumers finish using them.
  • Keeps the public DecodeFrame ownership model unchanged.
  • Uses canonical fast paths for common TCP Keep + Data frames.
  • Uses an atomic fast path for carrier closed-state checks.
  • Reuses fixed 8 KiB TCP uplink buffers through sync.Pool.
  • Avoids context watcher goroutines for non-cancelable contexts.
  • Avoids allocating a replacement session map when a carrier is permanently closed.
  • Implements direct WaitReadFrom support for packet sessions.
  • Avoids the additional UDP staging buffer and copy normally required by the generic packet-connection wrapper.
  • Uses structured netip.Addr values instead of repeated IP string conversions.
  • Adds a fast path for domain fields containing IP literals.
  • Avoids allocations in warmed-up UDP packet writes.
  • Preserves structured IP addresses in the internal pooled decoder instead of converting wire IPs to strings and parsing them again.
  • Avoids inactive deadline and close-channel select overhead in steady-state UDP/XUDP writes while preserving deadline reset and close-cause behavior.
  • Uses atomic read-mostly attachment lookup for server XUDP ingress and egress while retaining generation-safe rebinding.
  • Removes a redundant worker availability lock during pool session creation.
  • Formats logical TCP remote addresses lazily.
  • Uses a shared direct helper for returning internally owned payloads to the pool.
  • Keeps primary and dedicated packet pools independently synchronized.
  • Adds no limiter work to the default unlimited carrier hot path.
  • Uses lost-wakeup-safe broadcast notifications only when a configured physical-carrier limit is saturated.
  • Includes persistent benchmarks for codec, worker, session, pool, and packet operations.

Representative Results

Measured on an Apple M3 Max. Results are environment-dependent.

Benchmark Before After
Internal pooled frame decode ~996 ns/op, 8192 B/op, 1 alloc ~145 ns/op, 0 B/op, 0 allocs
UDP decode, deliver, and direct receive ~211 ns/op, 96 B/op, 4 allocs ~175 ns/op, 84 B/op, 3 allocs
Client UDP/XUDP packet write ~94 ns/op, 0 B/op, 0 allocs ~52 ns/op, 0 B/op, 0 allocs
Server UDP/XUDP packet write ~76 ns/op, 0 B/op, 0 allocs ~35 ns/op, 0 B/op, 0 allocs
TCP session lifecycle ~5361 ns/op, 10974 B/op, 23 allocs ~4.3 us/op, ~2.8 KiB/op, 20 allocs
Packet-session lifecycle ~698 ns/op, 1360 B/op, 7 allocs ~265 ns/op, 1328 B/op, 6 allocs
Carrier worker write ~141 ns/op ~130 ns/op
Server carrier write ~136 ns/op ~132 ns/op
Carrier shutdown ~88 ns/op, 256 B/op, 3 allocs ~60 ns/op, 208 B/op, 2 allocs
Pooled payload release ~12.3 ns/op ~10.1 ns/op
Pool packet-session churn ~1339 ns/op ~1309 ns/op

The internal pooled decoder is used by carrier workers. The public DecodeFrame function continues to return independently owned payload memory so external callers are not exposed to pooled-buffer lifetime requirements.

Testing

Protocol and Codec Coverage

  • Golden wire vectors based on Xray-core revision 0ee156e75c9546a713f6c88c0bd14f5ff953c567.
  • New, Keep, End, and KeepAlive frame tests.
  • IPv4, IPv6, domain, UDP, and XUDP codec tests.
  • XUDP Global ID wire-format tests.
  • Per-datagram target metadata tests.
  • 8 KiB TCP frame-chunking tests.
  • Malformed frame and unexpected EOF tests.
  • Public and internally pooled decode-path tests.

Session Behavior

  • Full-duplex TCP tests.
  • UDP datagram-boundary and destination-preservation tests.
  • Server-first protocol tests.
  • Read and write deadline tests.
  • Context cancellation tests.
  • Packet-session dial-context lifetime tests.
  • Non-cancelable context lifecycle tests.
  • Idempotent-close and single-End tests.
  • Carrier EOF and write-failure propagation tests.
  • Direct and copied WaitReadFrom behavior tests.
  • Pooled payload ownership and consumption tests.

Server Behavior

  • Native TCP multiplexing tests through the server runtime.
  • UDP packet boundary and target-preservation tests.
  • XUDP rebinding across carriers with Global ID preservation.
  • Duplicate session ID and per-carrier session-limit rejection tests.
  • Authenticated-user isolation for XUDP flows.
  • Stale attachment, expiry, and write-failure generation-safety tests.
  • Graceful carrier draining during server and listener shutdown.
  • Native handler tests through the shared sing-based listener path.
  • Listener shutdown tests that verify active mux.cool carriers are drained.

Pool Behavior

  • TCP and UDP/XUDP carrier reuse tests.
  • Shared TCP and packet-session pool tests.
  • Dedicated packet-pool tests.
  • Dedicated UDP/XUDP concurrency enforcement tests.
  • Active-session limit tests.
  • Lifetime-session limit and carrier-rotation tests.
  • Strict physical-carrier limit tests under concurrent session creation.
  • Shared physical-carrier budget tests across primary and dedicated packet pools.
  • Context cancellation while waiting for physical-carrier capacity.
  • Capacity release after failed carrier dials, carrier closure, rotation, and shutdown.
  • Reuse tests verifying that blocked callers wake when logical capacity becomes available.
  • Broadcast wake-up tests for multiple callers waiting on the same saturated carrier.
  • Idle-cleanup tests.
  • Cross-carrier XUDP rebinding tests.
  • Global ID preservation across carrier rebinding.
  • Concurrent multi-carrier stress tests.
  • Shutdown-order tests for both pools and the underlying proxy.

Configuration and UDP/443 Behavior

  • Default and custom configuration parser tests.
  • Invalid concurrency and policy validation tests.
  • smux and mux.cool conflict tests.
  • Default, custom, and invalid max-carriers configuration tests.
  • Default UDP/443 rejection tests.
  • Explicit UDP/443 allow tests.
  • Explicit UDP/443 skip tests.
  • Verification that rejection does not open a carrier.
  • Verification that skip delegates directly to the base proxy.

Performance Regression Coverage

Persistent benchmarks cover:

  • Frame encoding.
  • Standard and pooled frame decoding.
  • UDP frame decoding.
  • Carrier worker writes and shutdown.
  • TCP session uplink processing.
  • TCP session lifecycle and buffer reuse.
  • Packet-session lifecycle.
  • Packet-session writes.
  • Packet delivery through ReadFrom.
  • Direct and copied WaitReadFrom paths.
  • Pool packet-session churn.
  • Parallel pool packet-session churn.
  • Server carrier writes.
  • Server UDP/XUDP enqueue and packet-write paths.

Process-Level End-to-End Tests

The integration suite includes a physical-carrier limit test that:

  • Builds the current mihomo binary.
  • Starts separate mihomo VLESS server and client processes.
  • Enables mux.cool with max-concurrency: 8 and max-carriers: 2.
  • Places an independent TCP relay between the two cores to count actual accepted physical connections.
  • Opens 16 simultaneous logical TCP sessions, filling both carriers.
  • Starts 4 additional sessions and verifies that they wait instead of opening a third carrier.
  • Releases 4 logical slots and verifies that the blocked sessions complete through carrier reuse.
  • Runs a second full wave to verify continued reuse.
  • Sends and echoes 36 logical TCP sessions while observing exactly 2 physical carriers and a peak of 2 active physical connections.

Run it with:

go test -tags=integration \
  -run '^TestMuxCoolProcessMaxCarriers$' \
  -v ./transport/muxcool

It also includes a bidirectional cross-runtime compatibility matrix that builds and starts real mihomo and Xray-core processes and verifies:

  • mihomo client → Xray server over TCP.
  • Xray client → mihomo server over TCP.
  • mihomo client → Xray server using normal UDP without an XUDP Global ID.
  • Xray client → mihomo server using normal UDP without an XUDP Global ID.
  • mihomo client → Xray server using XUDP and a dedicated packet pool.
  • Xray client → mihomo server using XUDP and a dedicated packet pool.
  • SOCKS5 UDP association, datagram payloads, response destinations, and reverse traffic.
  • Native VLESS Mux-command handling for v1.mux.cool carriers.

Normal UDP and XUDP are exercised as distinct wire modes. The normal-UDP scenarios use a zero Global ID, while the XUDP scenarios use source identity and an 8-byte Global ID. Independent TCP, UDP, and DNS echo services verify traffic outside either proxy process.

The test uses XRAY_E2E_BINARY when provided. Otherwise, it builds Xray-core from XRAY_CORE_ROOT or from a sibling Xray-core checkout.

Run the compatibility matrix with:

go test -tags=integration \
  -run '^TestMuxCoolProcessXrayCompatibility$' \
  -v ./transport/muxcool

Validation Results

  • Full go test ./... suite passes.
  • The affected adapter/outbound and transport/muxcool packages pass under go test -race.
  • The complete six-scenario mihomo/Xray process matrix passes under go test -race -tags=integration.
  • The cross-runtime matrix passes across five consecutive non-race runs and two consecutive race-detector runs.
  • go vet ./adapter/outbound ./transport/muxcool passes.
  • git diff --check passes.
  • The macOS ARM64 production binary builds and starts successfully.
  • The process-level VLESS E2E passes with 36 logical sessions over exactly 2 physical carriers.
  • The process-level Xray compatibility matrix passes for TCP, normal UDP, and XUDP in both directions.
  • Loopback interface counters remain at zero input errors, output errors, and collisions during the process-level test.
  • transport/muxcool statement coverage: 82.9%.

Compatibility

The wire format and XUDP behavior are based on Xray-core revision:

0ee156e75c9546a713f6c88c0bd14f5ff953c567

The feature is configured under the mux.cool proxy key. Mihomo uses kebab-case option names:

mux.cool:
  max-carriers: 4
  xudp-concurrency: 4
  xudp-proxy-udp443: reject

The XUDP options correspond to Xray-core’s xudpConcurrency and xudpProxyUDP443 fields. max-carriers is a mihomo extension that places a strict total cap on physical TCP carriers.

For VLESS, mihomo encodes a carrier targeting v1.mux.cool:9527 with the native VLESS Mux command instead of an ordinary TCP destination request. This matches Xray-core's VLESS behavior and is covered by both unit tests and real-process tests in both client/server directions.

mux.cool is applied as a generic outbound wrapper after the base proxy is created. It therefore works with Trojan as well as VMess, VLESS, and other compatible stream-capable proxy adapters. On the server side, mihomo's Trojan listener uses the same sing-based special-destination handler as VMess and VLESS, so v1.mux.cool carriers are accepted natively.

For example:

proxies:
  - name: trojan-mux
    type: trojan
    server: example.com
    port: 443
    password: password
    sni: example.com
    udp: true
    mux.cool:
      enabled: true
      max-concurrency: 8
      max-connections: 128
      max-carriers: 4
      xudp-concurrency: 4
      xudp-proxy-udp443: reject

Both endpoints must understand mux.cool. A conventional Trojan server without the v1.mux.cool server handler cannot terminate these carriers. The cross-runtime process matrix exercises VLESS against Xray-core for TCP, UDP, and XUDP in both directions. Trojan support is provided by the generic outbound wrapper and the shared Trojan listener handler, but does not yet have a dedicated two-process Trojan E2E test.

Notes

Mux can introduce head-of-line blocking because multiple logical sessions share a TCP carrier. This affects both TCP streams and tunneled UDP/XUDP packets.

A dedicated UDP/XUDP packet pool isolates per-carrier TCP session capacity from long-lived packet sessions. Without max-carriers it may increase the number of physical connections; with max-carriers configured, both pools remain inside the shared cap.

Pooled payload buffers are limited to internal carrier processing and are returned only after their consumer finishes. Public codec callers retain the original independent-memory behavior.

The feature is opt-in and disabled by default. UDP/443 is rejected by default when mux.cool is enabled, matching Xray-core behavior.

@Jolymmiles Jolymmiles changed the title feat: add Xray Mux outbound feat: add mux.cool outbound Jul 16, 2026
@Jolymmiles

Copy link
Copy Markdown
Author

@KT-Yeh could you look pls?

@Jolymmiles

Copy link
Copy Markdown
Author

@wwqgtxx это тоже будет закрыто или хотя бы прочитано описание и код?

@wwqgtxx

wwqgtxx commented Jul 17, 2026

Copy link
Copy Markdown
Collaborator

It won't be closed for now, but it won't be merged in the short term either.

The changes in this PR are extensive and clearly largely AI-generated, requiring significant effort to review.

Furthermore, while the feature is interesting, its suitability for merging is open to debate:

The implementation here is heavily biased towards xray, whereas muxcool was actually originated by v2ray-core. Extensions shouldn't focus solely on a single provider; the implications for v2fly-core need to be considered equally.

From another perspective, neither core has seen meaningful modifications to muxcool in years, and its level of refinement lags far behind smux and HTTP/2-based multiplexing. Neither project's newly introduced protocols actually utilize muxcool. We remain unsure whether the ongoing maintenance costs for this feature are justified.

@Jolymmiles

Copy link
Copy Markdown
Author

It won't be closed for now, but it won't be merged in the short term either.

The changes in this PR are extensive and clearly largely AI-generated, requiring significant effort to review.

Furthermore, while the feature is interesting, its suitability for merging is open to debate:

The implementation here is heavily biased towards xray, whereas muxcool was actually originated by v2ray-core. Extensions shouldn't focus solely on a single provider; the implications for v2fly-core need to be considered equally.

From another perspective, neither core has seen meaningful modifications to muxcool in years, and its level of refinement lags far behind smux and HTTP/2-based multiplexing. Neither project's newly introduced protocols actually utilize muxcool. We remain unsure whether the ongoing maintenance costs for this feature are justified.

С учетом того, что в xray никто не трогает mux.cool не думаю, что потребуется вообще, что либо менять, а если возникнут ошибки, я сам посмотрю и подготовлю испралвения. Я понимаю, что есть красивый smux, но к сожалению xray несколько недальновидны и не занимаются повышением качества в данном аспекте

Comment thread listener/sing/sing.go
return h.muxService.NewConnection(ctx, conn, UpstreamMetadata(metadata))
case vmess.MuxDestination.Fqdn:
return vmess.HandleMuxConnection(ctx, conn, metadata, h)
return h.muxCool.Serve(ctx, conn, metadata, h)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❓ Question — Architecture

This call replaces vmess.HandleMuxConnection (sing-vmess's built-in mux.cool server) with the new muxcool.ServerRuntime. Since vmess.MuxDestination.Fqdn is the entry point for all vmess/vless inbound mux.cool traffic, every existing client that relied on sing-vmess's implementation will now hit the new code path.

Could you confirm interoperability was validated against Xray-core clients (both vmess and vless, with and without XUDP GlobalID)? The added TestInboundVless_MuxCool / TestInboundVMess_MuxCool cover mihomo-to-mihomo flows but I don't see a cross-implementation assertion in the PR.

olicesx

This comment was marked as duplicate.

olicesx

This comment was marked as duplicate.

olicesx

This comment was marked as duplicate.

@olicesx olicesx left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No blocking or major issues after a full pass. The layering is clean (codec -> session -> pool -> server), shutdown ordering and close-once discipline are handled carefully, and test coverage is thorough (codec/session/pool/E2E/benchmarks all present). A few minor suggestions and one interoperability question are listed below.

[details] Detailed review

Review matrix

Dimension blocking major minor nit
Security 0 0 0 0
Correctness 0 0 0 0
Performance 0 0 1 0
Architecture 0 0 1 (needs confirmation) 0
Maintainability 0 0 1 0
Docs 0 0 1 0
Test 0 0 0 0
Total 0 0 4 0

Merge recommendation: COMMENT (no blocking/major; merge after author evaluates the minor items)

Highlights

  • decodeFramePooled + sessionBufferPool keep frame processing allocation-free, verified by benchmarks
  • CarrierLimiter lets the TCP pool and XUDP pool share a single physical-carrier budget, preventing runaway resource use per proxy
  • serverPacketFlow's generation + currentFast design correctly handles XUDP flow concurrency across carrier rebinds; the inline note on "old-generation write completing after rebind" captures the key invariant clearly
  • parseVlessAddr detects the native mux command byte via the v1.mux.cool:9527 sentinel without polluting ordinary VLESS flows
  • Close paths carry explicit "defer close to avoid re-entry" comments (carrierWorker.writeFrame, serverCarrier.writeFrame), showing real thought about close fan-out re-entrancy

Open question

listener/sing/sing.go:114 replaces vmess.HandleMuxConnection with h.muxCool.Serve. Because vmess.MuxDestination.Fqdn is the entry point for all vmess/vless inbound mux.cool traffic, every existing client that depended on sing-vmess's built-in mux.cool implementation is now on the new path. Consider attaching Xray-core interoperability evidence (vmess+vless, with and without XUDP GlobalID) to the PR description.

Minor findings

  1. [Docs] docs/config.yaml -- mux.cool block is missing max-carriers
    The sample lists max-concurrency, max-connections, xudp-concurrency, and xudp-proxy-udp443, but max-carriers is parsed in adapter/parser.go (and documented in the PR body) without a corresponding example here. Adding a commented # max-carriers: 0 line would keep the sample consistent with the implemented options.

  2. [Architecture] listener/sing/sing.go:114 -- HandleMuxConnection replacement interoperability (needs confirmation)
    This call replaces vmess.HandleMuxConnection (sing-vmess's built-in mux.cool server) with the new muxcool.ServerRuntime. Because vmess.MuxDestination.Fqdn is the entry point for all vmess/vless inbound mux.cool traffic, every existing client that depended on sing-vmess's built-in implementation will now hit the new code path.
    Could you confirm interoperability was validated against Xray-core clients (both vmess and vless, with and without XUDP GlobalID)? The added TestInboundVless_MuxCool / TestInboundVMess_MuxCool cover mihomo-to-mihomo flows but I don't see a cross-implementation assertion in the PR.

  3. [Observability] transport/muxcool/server_packet.go:232 -- WritePacket silently drops when detached
    When attachment == nil (reusable XUDP backend detached between carriers), WritePacket returns nil and the downstream packet is silently dropped. The inline comment explains why this is correct for "late responses", but from the upper-layer NewPacketConnection handler's perspective the write appears successful, which can mask real packet loss during carrier rotation.
    Consider returning a non-error sentinel (e.g., ErrPacketDetached that the handler can explicitly ignore) or exposing a counter, so observability tooling can distinguish dropped late packets from genuinely successful sends.

  4. [Performance] transport/muxcool/pool.go:115 -- openContext does an O(N) worker scan per dial
    Pool.openContext walks every worker on each dial attempt. With max-carriers unset (default, unlimited) and many long-lived carriers accumulated, this becomes O(N) per DialContext/ListenPacketContext and serializes on p.mu.
    For the current default workload this is fine, but if max-carriers is raised or carriers accumulate under rotation, consider tracking the first available worker (or a free-list keyed by remaining capacity) to keep the common case O(1).

Noted but not posted

  • MaxPayloadSize = 8KB matches the Xray protocol value
  • OptionError semantics on End frames are symmetric with the client-side remoteFrameError
  • The fakeMuxCoolAdapter test stub is well-structured, covering carrier reuse, concurrency, error paths, and close-event ordering
  • ~5,000 lines of new tests (incl. benchmarks/E2E) are sizable but each item maps to a clear contract or performance dimension; no obvious redundancy

@olicesx

olicesx commented Jul 30, 2026

Copy link
Copy Markdown

Note: this review was submitted multiple times due to a tooling hiccup on my side. The canonical version is the latest COMMENTED review with ASCII-safe formatting (-> instead of arrows). Earlier duplicates contain mojibake from PowerShell JSON encoding. Apologies for the noise.

@olicesx

olicesx commented Jul 31, 2026

Copy link
Copy Markdown

A follow-up pass turned up a few more minor items -- mostly resource-cleanup gaps (the new ListenerHandler.Close leaves muxService open, and the packet/stream finish paths don't drain queued payloads back to the pool) plus one decode-side asymmetry on payload size. None of this blocks merge; the one thing worth a deliberate decision is whether the shared CarrierLimiter between the TCP and XUDP pools is the intended semantics. Items already raised in the prior five reviews are not repeated.

📋 Detailed analysis

Review matrix

Dimension blocking major minor nit
Security 0 0 1 0
Correctness 0 0 0 0
Resource hygiene 0 0 2 0
Defensive 0 0 1 0
Docs 0 0 1 0
Total 0 0 5 0

Merge recommendation: COMMENT (follow-up pass; no change to prior recommendation)

Additional points verified

  • Encode/decode symmetry holds for address-type bytes (1/2/3) and the GlobalID condition (New + UDP + Data); hasTarget is symmetric on both sides
  • releasePayload is called on every error path in decodeFrameWithPayloadPool and all server.go / packet_session.go consumption sites; the public DecodeFrame forces poolPayload=false, so the public path cannot leak
  • Max frame length = 2 + 512 + 2 + 65535 = 66051, well within int; no overflow on buffer[:frameLen]
  • Lock ordering is unidirectional (pool.mu -> worker.mu); callback paths release worker.mu before acquiring pool.mu, no reverse nesting; go w.close(err) deferred fan-out avoids reentrant deadlock
  • lease / release are paired on every path (dial failure, pool closed mid-dial, worker.close) via limitedCarrier.Close -> once.release
  • Outbound integration is purely additive: vless.go only adds one more condition for Mux=true and does not affect non-mux.cool traffic; smux/mux.cool mutual-exclusion check is in place (adapter/parser.go)

Open question

adapter/outbound/muxcool.go:92-96 passes the same CarrierLimiter instance to both the TCP pool and the xudpPool (TestCarrierLimiterIsSharedAcrossPools at transport/muxcool/pool_test.go:188 confirms this is intentional). Under XUDP-heavy traffic, TCP dials can be starved (and vice versa). Is this the intended trade-off (cap total physical carriers regardless of transport)? If so, documenting max-carriers as a "combined TCP+XUDP budget" would make the semantics explicit.

Minor findings

🔵 minor [Security] transport/muxcool/codec.go:309-316 -- decode accepts the full uint16 payloadLen range
The decode path accepts any payloadLen up to 65535 (full uint16 range), while the send side (transport/muxcool/packet_session.go:134) caps at MaxPayloadSize=8192. Impact: a malformed or hostile peer can push 64KB frames per read. Suggestion: for Xray wire-format compatibility this is acceptable (Xray also uses uint16), but a one-line comment noting that MaxPayloadSize only constrains the send side would prevent future confusion.

🔵 minor [Resource leak] listener/sing/sing.go:129-131 -- ListenerHandler.Close() closes muxCool but not muxService
The newly added Close only closes muxCool, leaving the sibling muxService (*mux.Service, listener/sing/sing.go:58) un-closed. Impact: the sing-mux service goroutines and state are not released on shutdown; listener/sing_vless/server.go and listener/sing_vmess/server.go inherit the same gap because they only call l.handler.Close(). This PR doesn't introduce a fresh leak (there was no Close before), but since the method is being added to fix a resource issue, completing the job here would be cleaner. Suggestion: add if h.muxService != nil { h.muxService.Close() } (or the equivalent API) before closing muxCool.

🔵 minor [Resource hygiene] transport/muxcool/packet_session.go:228 & transport/muxcool/session.go:166-175 -- finish doesn't drain queued payloads
Unlike transport/muxcool/server_stream.go:106-116 writeInput, which calls releaseQueued() on the <-s.done branch, the packet session's finish and the stream runDownlink exit without draining already-enqueued payloads. Impact: payloads eventually get GC'd, so this is not a permanent leak, but under high load it puts pressure on sync.Pool. Suggestion: align the drain pattern across all three paths (call a releaseQueued()-equivalent before returning on <-s.done).

🔵 minor [Defensive] transport/muxcool/server_stream.go:57-61 -- handler returning nil doesn't call finish
Currently relies on the handler eventually calling client.Close() to trigger readOutput's peer.Read -> EOF -> finish. Impact: all current handlers (e.g. listener/sing/sing.go:140 HandleTCPConn) do close the conn, but a future handler that violates this contract would leak three goroutines (writeInput / readOutput / handler). Suggestion: add defer s.finish(nil, false) (closeOnce already prevents re-entry) to make the path robust against handler misbehavior.

🔵 minor [Docs] adapter/outbound/muxcool.go:131-137 -- UDP/443 default reject not surfaced in docs
Defaulting to reject for UDP/443 (i.e. QUIC/HTTP3) is a significant behavior change when mux.cool is enabled, but neither docs/config.yaml nor the option comment mention that reject is the default. Impact: users enabling mux.cool will see QUIC traffic silently fail without realizing why. Suggestion: add a one-line note in docs/config.yaml (and/or the option struct comment) stating the default is reject and what it means for QUIC.

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.

3 participants