Skip to content

Commit e8cc45e

Browse files
committed
net/http: fix when writeLoop exited still has request in writech
When there is still a request in writech after writeLoop exits, the request body will not be closed and will not be retried. The request is not sent to the server, so it can be safely retried without regard to idempotence. Fixes golang#49621
1 parent a5a4744 commit e8cc45e

File tree

2 files changed

+138
-0
lines changed

2 files changed

+138
-0
lines changed

src/net/http/transport.go

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -719,6 +719,13 @@ func (pc *persistConn) shouldRetryRequest(req *Request, err error) bool {
719719
// can "rewind" the body with GetBody.
720720
return req.outgoingLength() == 0 || req.GetBody != nil
721721
}
722+
if len(pc.writech) != 0 {
723+
// Request was sent successfully into writech, but isn't read from it
724+
// because writeLoop exited.
725+
// In this case, you can retry without considering idempotence.
726+
// see https://github.com/golang/go/issues/49621
727+
return true
728+
}
722729
if !req.isReplayable() {
723730
// Don't retry non-idempotent requests.
724731
return false

src/net/http/transport_test.go

Lines changed: 131 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ import (
4040
"strings"
4141
"sync"
4242
"sync/atomic"
43+
"syscall"
4344
"testing"
4445
"testing/iotest"
4546
"time"
@@ -6654,3 +6655,133 @@ func testHandlerAbortRacesBodyRead(t *testing.T, mode testMode) {
66546655
}
66556656
wg.Wait()
66566657
}
6658+
6659+
type issue49621Listener struct {
6660+
net.Listener
6661+
}
6662+
type issue49621Conn struct {
6663+
net.Conn // base
6664+
6665+
mtx sync.Mutex
6666+
err error
6667+
n int
6668+
}
6669+
6670+
func (l *issue49621Listener) Accept() (net.Conn, error) {
6671+
c, err := l.Listener.Accept()
6672+
if err == nil {
6673+
c = &issue49621Conn{Conn: c}
6674+
}
6675+
return c, err
6676+
}
6677+
6678+
func (c *issue49621Conn) inject(n int) bool {
6679+
c.mtx.Lock()
6680+
defer c.mtx.Unlock()
6681+
c.n += n
6682+
return c.n > 1000
6683+
}
6684+
6685+
func (c *issue49621Conn) isFailed() error {
6686+
c.mtx.Lock()
6687+
defer c.mtx.Unlock()
6688+
return c.err
6689+
}
6690+
6691+
func (c *issue49621Conn) markFailed(err error) error {
6692+
c.mtx.Lock()
6693+
defer c.mtx.Unlock()
6694+
c.err = err
6695+
return c.err
6696+
}
6697+
6698+
func (c *issue49621Conn) Read(b []byte) (n int, err error) {
6699+
if err = c.isFailed(); err != nil {
6700+
return 0, err
6701+
}
6702+
6703+
n, err = c.Conn.Read(b)
6704+
if err != nil {
6705+
c.markFailed(err)
6706+
return n, err
6707+
}
6708+
6709+
if c.inject(n) {
6710+
return 0, c.markFailed(syscall.ECONNRESET)
6711+
}
6712+
6713+
return n, nil
6714+
}
6715+
6716+
func (c *issue49621Conn) Write(b []byte) (n int, err error) {
6717+
if err = c.isFailed(); err != nil {
6718+
return 0, err
6719+
}
6720+
6721+
if c.inject(len(b)) {
6722+
return 0, c.markFailed(syscall.ECONNRESET)
6723+
}
6724+
6725+
n, err = c.Conn.Write(b)
6726+
if err != nil {
6727+
return n, c.markFailed(err)
6728+
}
6729+
return n, nil
6730+
}
6731+
6732+
// Issue 49621: request not retry or close body after writeLoop exited.
6733+
func TestIssue49621(t *testing.T) {
6734+
ln := &issue49621Listener{newLocalListener(t)}
6735+
defer ln.Close()
6736+
6737+
addr := ln.Addr().String()
6738+
handler := HandlerFunc(func(w ResponseWriter, r *Request) {
6739+
if r.Method != MethodPost {
6740+
w.WriteHeader(StatusMethodNotAllowed)
6741+
return
6742+
}
6743+
6744+
_, _ = io.Copy(io.Discard, r.Body)
6745+
_ = r.Body.Close()
6746+
w.WriteHeader(StatusOK)
6747+
})
6748+
s := &Server{
6749+
Addr: addr,
6750+
Handler: handler,
6751+
}
6752+
go func() { s.Serve(ln) }()
6753+
6754+
for i := 0; i < 10; i++ {
6755+
testIssue49621Request(t, addr)
6756+
}
6757+
s.Close()
6758+
}
6759+
6760+
func testIssue49621Request(t *testing.T, addr string) {
6761+
var wg sync.WaitGroup
6762+
6763+
for i := 0; i < 50; i++ {
6764+
wg.Add(1)
6765+
go func() {
6766+
defer wg.Done()
6767+
6768+
body := bytes.NewBuffer([]byte(`
6769+
hello world hello world hello world hello world
6770+
hello world hello world hello world hello world
6771+
hello world hello world hello world hello world
6772+
hello world hello world hello world hello world
6773+
hello world hello world hello world hello world
6774+
`))
6775+
6776+
resp, err := Post(addr, "plain", body)
6777+
if err == nil {
6778+
_, _ = io.Copy(io.Discard, resp.Body)
6779+
_ = resp.Body.Close()
6780+
return
6781+
}
6782+
6783+
t.Errorf("resp = %v, err = %v\n", resp, err)
6784+
}()
6785+
}
6786+
wg.Wait()
6787+
}

0 commit comments

Comments
 (0)