net/http: body argument to NewRequest is closed multiple times

[erikdubbelboer]

When passing an ReadCloser as body to http.NewRequest, it is possible the Close() of the ReadCloser is called multiple times for the same request. I'm wondering if this is allowed to happen, in which case it should be documented. Or if this is a bug?

it only seems to happen in rare cases where the full body is send (first close) but there is an error reading the response (second close).

Here are the stack traces which both called Close() on the same ReadCloser:

goroutine 1475 [running]:
runtime/debug.Stack(0x18, 0xc420d17ce0, 0x0)
        /usr/local/go/src/runtime/debug/stack.go:24 +0x79
github.com/mailru/easyjson/buffer.(*readCloser).Close(0xc4244fe140, 0xb2b600, 0x7fb44aacbf80)
        /home/ubuntu/src/github.com/mailru/easyjson/buffer/pool.go:263 +0x211
net/http.(*transferWriter).WriteBody(0xc424502180, 0xad2c40, 0xc422ca3480, 0x2, 0x2)
        /usr/local/go/src/net/http/transfer.go:332 +0x427
net/http.(*Request).write(0xc4244f8d00, 0xad2c40, 0xc422ca3480, 0x100000000410600, 0x0, 0x0, 0x0, 0x0)
        /usr/local/go/src/net/http/request.go:622 +0x6e9
net/http.(*persistConn).writeLoop(0xc422cd6240)
        /usr/local/go/src/net/http/transport.go:1707 +0x1ad
created by net/http.(*Transport).dialConn
        /usr/local/go/src/net/http/transport.go:1118 +0xa5a


goroutine 3775 [running]:
runtime/debug.Stack(0x0, 0xc424500c00, 0x304)
        /usr/local/go/src/runtime/debug/stack.go:24 +0x79
github.com/mailru/easyjson/buffer.(*readCloser).Close(0xc4244fe140, 0xc4244f8d00, 0xc421a7acc8)
        /home/ubuntu/src/github.com/mailru/easyjson/buffer/pool.go:261 +0xeb
net/http.(*Request).closeBody(0xc4244f8d00)
        /usr/local/go/src/net/http/request.go:1295 +0x43
net/http.(*Client).Do.func1(0xad2fc0, 0xc420011560, 0x0, 0x0)
        /usr/local/go/src/net/http/client.go:507 +0x59
net/http.(*Client).Do(0xc420106060, 0xc4244f8d00, 0x5f5e100, 0xc4244dd920, 0x7fb446aac5a0)
        /usr/local/go/src/net/http/client.go:602 +0x3d2
github.com/atomx/core/httpq.Do.func1(0xc4244f8d00, 0x5f5e100, 0xc4244dd860)
        /home/ubuntu/src/github.com/atomx/core/httpq/http.go:51 +0xe2
created by github.com/atomx/core/httpq.Do
        /home/ubuntu/src/github.com/atomx/core/httpq/http.go:56 +0x71

[bradfitz]

Yeah, I feel like I've seen this too, or that there was an existing comment on a CL or a bug or TODO about this. Thanks for filing.

[nerdatmath]

I'm taking a look at this.

[nerdatmath]

I have a test that replicates the double close.

[gopherbot]

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

[nerdatmath]

I have a fix. Check it out and let me know what you think.

[odeke-em]

@nerdatmath please check the CL, I think one more round to fix some docs then we'll be gucci.

[nerdatmath]

I'm traveling at the moment unfortunately. Can it wait about 9 days?

…

On Sun, Jun 25, 2017, 06:40 Emmanuel T Odeke ***@***.***> wrote: @nerdatmath <https://github.com/nerdatmath> please check the CL, I think one more round to fix some docs then we'll be gucci. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#19186 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AClAHkArM9kHTmR-AuKQTmf87eIi3NGEks5sHeS1gaJpZM4MFYOM> .

[bradfitz]

@nerdatmath, no, 9 days is too late. But somebody can take it over before then. Ideally in the next 24 hours. @odeke-em?

[nerdatmath]

Working on it right now...

[davecheney]

Why not permit it? What is the harm in allowing close to be called multiple times, the error is ignored anyway.

[bradfitz]

Why not permit it? What is the harm in allowing close to be called multiple times, the error is ignored anyway.

Because the io.Closer interface does not specify how implementations must behave on double Close. In fact, it says:

https://golang.org/pkg/io/#Closer

The behavior of Close after the first call is undefined.

So the conservative thing to do as a user of an io.Closer is not ever call it twice.

The second call might panic or deadlock.

[davecheney]

Fair enough.

…

On Mon, 26 Jun 2017, 11:12 Brad Fitzpatrick ***@***.***> wrote: Why not permit it? What is the harm in allowing close to be called multiple times, the error is ignored anyway. Because the io.Closer interface does not specify how implementations must behave on double Close. In fact, it says: https://golang.org/pkg/io/#Closer The behavior of Close after the first call is undefined. So the conservative thing to do as a user of an io.Closer is not ever call it twice. The second call might panic or deadlock. — You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub <#19186 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAAcA6-cEMK_RFl5DMvJ4BfAbO9jhLMNks5sHwWXgaJpZM4MFYOM> .