#2381·go-git

dumb HTTP transport: unnecessary re-downloads and dropped keep-alive connections

Author: stysusCreated Sep 11, 2026Updated Sep 11, 2026

Follow-up to #2380. While auditing plumbing/transport/http we also found several efficiency/connection-reuse regressions in the dumb-protocol path — not fetch-breaking, but they make dumb-HTTP fetches noticeably more expensive than they need to be, and they mirror bugs already fixed on the smart-HTTP path.

1. handshakeDumb doesn't drain the response body before closing

plumbing/transport/http/handshake.go (~line 347): uses a plain defer resp.Body.Close(), unlike handshakeSmart/checkError, which drain the body before closing so net/http can pool the underlying connection.

If packp.InfoRefs.Decode fails partway through the dumb /info/refs body (malformed/truncated response, or an HTML sign-in page from a captive portal/proxy), the deferred close runs on a partially-read body. Per net/http's connection-pooling contract (and this package's own drainAndClose doc comment describing exactly this hazard), the connection is dropped instead of returned to the pool, forcing a fresh TCP/TLS handshake for the next request.

This is the same class of bug fixed for the smart path in a prior commit (e959c317, "Discard the spent advertisement") — the dumb path was left unfixed.

Suggested fix: use drainAndClose(resp.Body) in handshakeDumb, consistent with handshakeSmart.

2. Pack indexes are re-decoded from disk on every pack-search fallthrough instead of cached

plumbing/transport/http/dumb.go:331: every time a loose-object lookup falls through to the pack search, all known .idx files are reopened and fully re-decoded into a new idxfile.MemoryIndex and appended to indicies, rather than decoding each index once and reusing the cached copy.

For a repository with M loose-missing objects across P packs, this does up to M×P full index-file opens/decodes/hasher allocations instead of P total, and indicies grows with duplicate decoded indices for the life of the fetch — a real slowdown and memory-growth risk on larger fetches.

Suggested fix: decode each pack index once (e.g. keyed by pack hash) and reuse the cached idxfile.MemoryIndex on subsequent lookups instead of re-decoding.

3. resolveClient isn't cached, so keep-alive connections are discarded across operations

plumbing/transport/http/http.go:253: builds a fresh *http.Transport/*http.Client (via http.DefaultTransport.Clone()) on every call when Options.Client is nil, and it's called fresh inside every Handshake (handshake.go:118) rather than being cached once on the Transport struct.

A *Transport is typically constructed once and reused across multiple Fetch/Push/Clone calls on the same remote (see remote.go, which calls Handshake on the same client at multiple call sites). Each such call currently discards any pooled/keep-alive TCP+TLS connections from the previous operation against the same host, paying a fresh handshake per git operation instead of amortizing connection reuse across operations in a session.

Suggested fix: lazily build and cache the resolved *http.Client once per Transport (guarded appropriately if Transport can be used concurrently) instead of rebuilding it on every Handshake.


Happy to send a PR for any/all of these if useful.


Disclosure: these findings came from an AI-assisted code review (Claude) of this package, run alongside the investigation for #2380. The core claims (missing drainAndClose in handshakeDumb, repeated pack-index decoding, and resolveClient not being cached on Transport) were manually spot-checked against the current source before filing. Happy to provide more detail or a reproduction if useful.