[xianyu-sheng]

fix: release per-connection dump goroutines by closing the pipe writers

Problem

handleClientMessage and handleServerMessage each create an io.Pipe, hand
its writer to the copy path and start a protocol-dump goroutine that reads from
the pipe:

r, w := io.Pipe()
tee := io.MultiWriter(c.svrConn, w)
go protocol.CreateInterop(settings.Protocol).Dump(r, protocol.ClientSide, c.id, settings.Quiet)
c.copyDataWithRateLimit(tee, c.cliConn, protocol.ClientSide, settings.UpLimit)

The writer is never closed. When the proxied connection ends, copyData
returns and c.stop() closes both network connections, but the dump goroutine
stays blocked in r.Read(...) forever — an io.Pipe reader only observes EOF
when the writer is closed. Every proxied connection leaks two goroutines.

Reproduction

conn_test.go starts tproxy in-process against a local echo backend, opens and
closes 5 connections, then compares runtime.NumGoroutine() with the baseline.

Before:

goroutines baseline=6 after=16 connections=5
--- FAIL: TestConnectionGoroutinesAreReleased

After closing the pipe writer when the handler returns:

goroutines baseline=6 after=6 connections=5
--- PASS: TestConnectionGoroutinesAreReleased

go test -count=3 passes.

Detection

Found during a SmartBench audit of
the repository. Its evidence-gated LLM review on the vulnerable revision flags
the pipe writer handling at conn.go:64-82 and recommends defer w.Close();
the reproduction above confirms the finding independently of the audit.

Thanks!

[xianyu-sheng]

Thanks for the review so far.

I'm preparing a public dataset of redacted audit trajectories from SmartBench and would like to include this case.

The record would contain only analysis/rule IDs, project-relative file paths and line numbers, the finding summary, and this public thread URL. It would not include raw prompts/responses, secrets, or absolute paths. Nothing is published without your explicit consent.

If you're comfortable with that, please reply with:

"I, , consent to SmartBench publishing a redacted audit trajectory for kevwan/tproxy under CC BY 4.0."

If you'd prefer not to be included, just say so — we won't publish it and will delete our local copy. No response also means it stays unpublished. We can list the repository on the data-contributor leaderboard only if you consent; tell us if you prefer a display name or no listing.

Policy: https://github.com/xianyu-sheng/SmartBench/blob/main/trajectories/CONSENT.md

[kevwan]

The goroutine leak is worth fixing, but this PR needs revision before merging: closing the writers exposes EOF handling bugs in several supported protocol dumpers.

1. P1: Redis, MySQL, and MQTT dump goroutines spin on EOF. The new writer closes in conn.go:69 and conn.go:80 make subsequent pipe reads return EOF immediately. Redis ignores read errors; MySQL and MQTT continue looping on EOF. After a connection closes, these dump goroutines therefore remain alive and consume CPU continuously, instead of remaining blocked as before. A bounded reproduction using a closed pipe confirmed 1,000 consecutive EOF reads without returning for each of these three dumpers. Please make them exit on EOF and extend the regression coverage to these protocols.

2. P2: The regression test leaves background work running across runs. At conn_test.go:55, the listener is started without a shutdown path, and the goroutine-count allowance permits connection cleanup to outlive the test. Running go test -race -count=3 -timeout=45s ./... detected a race between a later run assigning global stat in startListener() and an earlier connection's stop() reading it. Please use a test-owned listener, stop the background work, and explicitly wait for connection handlers to finish before returning.

Validation: reviewed head f5a13ca5f381427c31eabd883f3da5706ac65b37 against base 817e866b1553f64d19edfbc9377e27bb51a97692 in an isolated checkout on macOS. The added test passed in a single race-enabled run; the repeated race-enabled run and the bounded EOF reproduction failed as described. The current regression test exercises only the default protocol.