Also arguably related to this: the request body is not necessarily closed before Do returns.

Okay, this is more contrived than I originally thought. What's happening is that your reader in this case is infinite, or at least longer than the server's handler is reading. Because the server handler returns before reading to EOF, it sets "Connection: close" and interrupts the TCP connection while the client is still writing in writeLoop.

So, yes, we could wait for the writer to be done writing at body EOF (if there's a body) or before returning the body-less response headers.

CL https://golang.org/cl/17312 mentions this issue.

The more I work on this, the less I think it's fixable. The change https://golang.org/cl/17312 kinda fixes it, at least for some cases, but ultimately it's not possible to cancel a blocked Read call, which means that without heuristics like "wait 100ms", RoundTrip can deadlock if the server replies before the request body is written and the request body is still being written from a forever-blocked Reader.

I think I'd prefer to fix this with documentation, updating the RoundTripper docs to say that the body may still be being Read and Closed after RoundTrip returns, and that callers who want to reuse the Body have to wait on it themselves. (e.g. with a wrapped ReadCloser noting when it's done and providing synchronization)

FWIW this is worse than I thought originally. Just closing the body after the request is enough to trigger the race. For example, the race detector shows a data race in the following innocuous looking code which tries to PUT a body from an open file, then closes the file.

It's common defensive practice to "defer close" a file - this code looks alright even knowing that Post will close the file, because closing a file twice isn't a problem. The fact that net/http asynchronously closes the file after the Do has completed is unfortunate.

package main

import (
    "log"
    "net/http"
    "net/http/httptest"
    "os"
)

const largeFile = "/mnt/syn/rog/movies/TheLongHope.mov"

func main() {
    srv := httptest.NewServer(http.HandlerFunc(http.NotFound))
    f, err := os.Open(largeFile)
    if err != nil {
        log.Fatal(err)
    }
    defer f.Close()
    resp, err := http.Post(srv.URL, "text/plain", f)
    if err != nil {
        log.Fatal(err)
    }
    log.Printf("resp: %v", resp.Status)
}