Skip to content

fix: cancel abandoned SOCKS UDP setup - #31

Merged
cppla merged 1 commit into
mainfrom
codex/socks-udp-setup-cancellation
Sep 23, 2026
Merged

cppla merged 1 commit into
mainfrom
codex/socks-udp-setup-cancellation

Conversation

@cppla

@cppla cppla commented Sep 23, 2026 •

Copy link
Copy Markdown
Owner

Summary

  • Monitor the SOCKS UDP control connection from after complete request parsing through endpoint lookup, packet dialing and established forwarding. EOF cancels this request's setup wait, so abandoned setup does not retain local admission until the upstream timeout.
  • Keep one control reader and join it on every exit; join the two packet workers before ordinary tracked connection release. Successful setup cancels its timer without terminating the UDP session.
  • Own non-nil returned packet connections even during cancellation races or packet-plus-error returns; bound both success and failure replies with the existing handshake write budget.

Verification

  • Reproduced the old behavior on the base with real loopback native QUIC and a non-responding peer: after downstream close, the sole slot remained occupied and a new greeting was reset until the fixture's upstream timeout.
  • New production-native regression waits for a real QUIC Initial, closes the SOCKS control connection, checks caller cancellation and admission release, and accepts a fresh greeting. Passed race x10; no manual transport invalidation or shared-dial cancellation.
  • Controlled regressions cover full/half-close, late-success races, exactly-once packet Close, setup timer detachment with real UDP echo beyond the original deadline, live-peer failure cleanup, forced shutdown, and packet-worker joining.
  • Helper/DNS validation, fragmented request parsing, real pipe write-deadline reply stalls, existing UDP source restrictions, queue/target-error continuity and native shared-dial isolation passed.
  • Full make check, full make race, targeted race x5 and independent review passed. Single-change negative controls fail the intended cancellation, timer-cleanup and packet-ownership assertions.
  • Linux/arm64 isolated Docker: 23 top-level cases x3, 69 passes. All 88 proxy/tunnel Go source hashes were unchanged across compile and execution. Container cleanup verified; no image pull/prune/restart.
  • Exact head d99f9b160493320969cecb1e5d2febec484d4e6b passed CI (all 11 jobs), CodeQL, and Linux namespace/netem. Actual Docker integration, OCI export and netem steps and success log markers were checked. No open review threads at merge verification.

Boundaries

Only this caller's setup lifetime is changed. Native shared physical dialing remains client-owned; H3 keeps its existing distinct cold-dial retry policy. TCP CONNECT handling and graceful/forced shutdown contracts are unchanged. Custom resolvers/dialers that ignore context cannot be forcibly interrupted. The DNS fixture checks watcher-to-validation context propagation, not a full platform resolver integration experiment.

No dependency, wire format, release or tag change. This source-build fix is not in v1.0.1. These are correctness/resource-lifecycle checks, not throughput or browser-similarity experiments.

Merge receipt

Merged as dc0a714ae319394359f647e4b216fb5e89c711c8. Its tree 1981c16150d5a2c03b293100225c8df38bc93908 is identical to the tested PR head. Exact-main CI (all 11 latest job results), CodeQL, and Linux namespace/netem passed. Main Docker integration/OCI export and netem success logs were verified.

CI transparency: main CI attempt 1 failed only the unchanged TestQUICPacingRecoversAfterReceiverBackpressure on macOS at its fixture's aggregate Write-vs-Wait time assertion; the proxy package passed. The failure log is retained. There is no tunnel/accel source diff in this PR. The unchanged pacing test subsequently passed 10 local macOS repetitions, and a failed-jobs-only CI rerun passed (attempt 2). This does not establish that the timing-based fixture assertion is reliable; replacing it with causal backpressure evidence remains separate follow-up work. No threshold was relaxed and no test was skipped here.

Local main is synchronized and clean. No release/tag was created.

Copilot AI lite review requested due to automatic review settings September 23, 2026 03:38
@cppla
cppla merged commit dc0a714 into main Sep 23, 2026
15 checks passed

Copilot AI 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.

Copilot review overview

🟡 Changes recommended

A moderate deadline-aware context-check issue remains at the setup guards.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 1 Medium severity

Open (1)
What changed in this PR

Fixes abandoned SOCKS5 UDP setup cancellation and resource cleanup when the control connection closes.

Changes:

  • Cancels setup across lookup, dialing, and forwarding.
  • Safely joins readers/workers and owns packet connections.
  • Adds regression tests and documents lifecycle behavior.
File Reviewed changes
internal/​proxy/​socks5.go Setup cancellation and worker lifecycle handling.
internal/​proxy/​socks5_udp_test.go Validation and admission-release coverage.
internal/​proxy/​socks5_udp_setup_native_test.go Native QUIC cancellation regression coverage.
internal/​proxy/​socks5_udp_setup_cancel_test.go Cancellation races, ownership, timers, and shutdown.
internal/​proxy/​socks5_udp_control_test.go Control-reader lifecycle and reply-stall behavior.
docs/​DEPLOYMENT.md SOCKS UDP cancellation semantics and boundaries.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread internal/proxy/socks5.go
Comment on lines +208 to 211
if err := ctx.Err(); err != nil {
_ = reply(socksReplyForError(err), nil)
return
}
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.

2 participants