golang/go

net/http: (*Transport).getConn traces through stale contexts

Open

#21,597 opened on Aug 24, 2017

View on GitHub
 (6 comments) (0 reactions) (0 assignees)Go (19,008 forks)batch import
NeedsFixThinkinghelp wanted

Repository metrics

Stars
 (133,883 stars)
PR merge metrics
 (No merged PRs in 30d)

Description

(*http.Transport).getConn currently starts a dialConn call in a background goroutine: https://github.com/golang/go/blob/0b0cc4154e1defed07e73ca1304b9a68c7134577/src/net/http/transport.go#L942-L945

That records traces to the provided Context and eventually invokes t.DialContext with it: https://github.com/golang/go/blob/0b0cc4154e1defed07e73ca1304b9a68c7134577/src/net/http/transport.go#L1029 https://github.com/golang/go/blob/0b0cc4154e1defed07e73ca1304b9a68c7134577/src/net/http/transport.go#L1060

This is pretty much a textbook illustration of the problem described in #19643 (Context API for continuing work). If (*Transport).getConn returns early (due to cancellation or to availability of an idle connection), the caller may have already written out the corresponding traces, and dialConn (and/or the user-provided DialContext callback) will unexpectedly access a Context that the caller believes to be unreachable.

httptrace.ClientTrace says, "Functions may be called concurrently from different goroutines and some may be called after the request has completed or failed." However, that is not true of Context instances in general: if the http package wants to save a trace after a call has returned, it should call Value ahead of time and save only the ClientTrace pointer. If dialConn calls a user-provided DialContext function, then getConn should cancel the Context passed to it and wait for DialContext to return before itself returning.


See also #20617 (Context race in http.Transport).

Contributor guide