refactor: build one HTTP stack per endpoint instead of per connection

[jonstacks]

Summary

Stacked on top of the leak fixes. Base branch is stacks/remote-connection-bug. Review that PR first; this diff is only the last commit.

The two leaks fixed in the base PR were symptoms. The cause is that httpServe built an entire HTTP stack per inbound edge connection:

• an http.Transport — and therefore an upstream connection pool
• a httputil.ReverseProxy
• an http.Server
• a chanListener shim to feed that server its single connection
• and the goroutines behind all of it

…all discarded when the connection ended. This PR removes that shape: one stack per endpoint, built on the first forwarded HTTP connection and torn down when the forward loop exits.

Why per-connection was the wrong shape

Two signals, both pointing the same way:

1. net/http documents http.Transport as something to reuse rather than create as needed. It is safe for concurrent use and it is the thing that holds the connection pool. Constructing one per connection is close to the single most explicit "don't" in the package docs.

2. internal/httpx was written to be shared. chanListener has a 64-slot buffered connection channel and a sync.Map of per-connection metadata keyed by conn, and Close() ranges over all live connections. You only need a buffer and a map if you expect many connections. ServeConnServer exists to work around the absence of http.Server.ServeConn (proposal: net/http: add a ServeConn method to http.Server to handle net.Conn golang/go#36673) precisely so that one server can serve individually-accepted connections. It was being used one-server-per-connection.

The cost beyond the leak

Because the connection pool was thrown away every time, no inbound connection could ever reuse an upstream connection. Every single one paid a fresh TCP — and for https upstreams, TLS — handshake to the upstream. For an operator forwarding to a cluster service, this is likely the more expensive half of the bug.

Measured with the offline harness:

	before	after
Upstream connections for 25 sequential inbound connections	25	1
Upstream connections for 300 concurrent inbound connections	300	~43
Live upstream connections after endpoint teardown	leaked	0
Goroutine growth over 50 inbound connections	+50	0

What made sharing possible

The blocker was buildHTTPTransport baking ServerName into the TLS config from the upstream URL. That pins a transport to whichever upstream happened to be configured when it was built, so it cannot be shared across an upstream change.

Leaving ServerName unset fixes this: http.Transport derives SNI per request from the request URL. Verified before committing to the design — one shared transport correctly sent alpha.test then beta.test, and pooled a third request onto the first connection.

One trap worth flagging for review. The tempting next step is to drop the TLS config entirely and pass nil. Don't. A nil TLSClientConfig (with no custom dialer and ForceAttemptHTTP2 false) makes net/http enable automatic HTTP/2 negotiation for https upstreams — a protocol change nobody asked for. The config has to stay non-nil but empty. There is a regression test pinning this.

Behavior changes

Two, both intentional:

1. UpdateUpstream now takes effect on the next request, not the next connection.

The shared reverse proxy reads e.upstreamURL.Load() per request rather than capturing it per connection. This is the more useful behavior: ngrok's edge can hold an inbound connection open long enough that a per-connection snapshot would keep routing to a stale upstream well after the caller retargeted it. Requests already in flight are unaffected either way.

The UpdateableEndpointForwarder doc comment is updated to match, and there is a test that pins the new semantics over an already-open keep-alive connection. This is user-visible and should be called out in the changelog.

2. Endpoint teardown now closes in-flight forwarded connections rather than letting them drain.

closeHTTPStack calls server.Close(), which closes active connections. Previously each connection owned its own server and was unaffected by endpoint teardown. The tunnel carrying those connections is going away at that point regardless, so this seemed right — but it is a judgement call and worth a second opinion.

Also included

• ReadHeaderTimeout on the server, which previously had no timeouts at all.
• MaxIdleConnsPerHost: 100. The stdlib default is 2, which is very low for something whose whole job is forwarding to one upstream. This is a tuning knob rather than correctness — easy to drop or change if you'd rather not pick a number here.
• BaseContext returning the endpoint's context, the idiomatic way to give handlers a server-lifetime context.

Notes for review

• The stack is built lazily behind a mutex rather than eagerly in start(), for two reasons: an endpoint that only forwards raw TCP should not spawn an HTTP server goroutine, and isHTTP() depends on the mutable upstream URL, so UpdateUpstream can switch an endpoint between HTTP and TCP at any point.
• httpServe closes the connection and returns if the endpoint is already torn down, rather than resurrecting a stack during shutdown.
• defer stop() from the base PR's httpx change is load-bearing here. Threading a real cancelable context into ServeConn activates the second leak mode described in that PR, so this change depends on it.

Verification

go build ./... && go vet ./...
go test -race -count=3 ./...        # green
go build -C ./examples ./...        # green

Full online integration suite passes with NGROK_TEST_ONLINE=1, including TestWebSocketUpgrade.

New tests cover upstream connection reuse, 300-way concurrency against the 64-slot chanListener buffer, WebSocket upgrade through the shared stack, UpdateUpstream over an open connection, and the two transport-configuration guards. Each was confirmed to fail against a per-connection stack (25 sockets, +100 goroutines) before passing here.

🤖 Generated with Claude Code