Repository navigation
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
docker-agent
left a comment
There was a problem hiding this comment.
Assessment: 🟢 APPROVE
full diff: moby/moby@client/v0.6.1...client/v0.6.2 Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
docker-agent
left a comment
There was a problem hiding this comment.
Assessment: 🟢 APPROVE
The key change in vendor/github.com/moby/moby/client/hijack.go adds a context.AfterFunc-based cancellation mechanism to setupHijackConn. The named-return signature change (retConn net.Conn, mediaType string) is required to allow the deferred handler to override return values, and the core design is correct.
The concurrent-close pattern (AfterFunc closes hc.Conn while rt.RoundTrip is running) is safe: TLS handshake completes inside the dialer before AfterFunc is registered, and both net.TCPConn and tls.Conn explicitly support concurrent Close() with ongoing reads/writes as a documented Go networking pattern. The <-cancelled channel drain and named-return override correctly handle the cancellation-wins-the-race case.
Lower-confidence findings (not posted inline)
These low-severity observations were not verified and are informational only:
- [low]
vendor/github.com/moby/moby/client/hijack.go:57 — When cancellation wins, the pre-existingdefer conn.Close()(line 57) will also fire sinceretErris set, resulting in a benign double-close of the same underlying connection.net.Conn.Close()on an already-closed connection discards the error silently (_ = conn.Close()), so this is harmless in practice. - [low]
vendor/github.com/moby/moby/client/hijack.go:83 —ctx.Err()is theoretically nil if a custom/misbehaving context implementation does not set it when cancelled, which would yield a nil connection with no error. Standard library contexts always return non-nil once done; this is not a concern for the standard usage.
|
replaced by #7368 |
full diff: moby/moby@client/v0.6.1...client/v0.6.2
Summary
Release notes (optional)
A picture of a cute animal (not mandatory but encouraged)