Close connections leaked on setup error paths

Alternative to !6953 (closed). Credit to @kkyrala for finding and diagnosing the leak.

Root cause

Connecting to the docker daemon runs four validation checks after the connection is established. When one of them failed, the connection was never closed: the executor only takes ownership after all checks pass, so cleanup never saw it. For the docker-autoscaler and instance executors the connection wraps a live SSH tunnel, and every failed preparation attempt stranded the tunnel's goroutines for the life of the manager process.

The fix

Instead of closing at each error return, close once in a deferred check on the named return error, so error paths added to connectDocker later stay covered. The same call chain had two more places that leaked the tunnel client on failure, in the connection constructor and in the dialer setup. Both now clean up what they own before returning an error.

Same bug class elsewhere

A sweep of the codebase for the same pattern (resource created, fallible step, error return without close) found two more instances, fixed here in separate commits:

  • The fleeting instance and nesting VM tunnel dials check the context cause before the dial error. A dial that succeeds concurrently with job cancellation was dropped without close, stranding the same SSH keepalive goroutines. Cancellation while dialing a slow instance is a common combination.
  • The session server bound its TCP listener before validating the advertise address, leaking the bound socket on validation failure. Validation now runs first. Cosmetic in practice since the only caller exits on error.

There is no regression test because the connection factory is hard-wired into the connect path. Making it injectable is a follow-up.

Edited by Igor

Merge request reports

Loading