Skip to content

Commit

Permalink
Revert an upstream Go change that broke us.
Browse files Browse the repository at this point in the history
See golang/go#60818

Updates tailscale/corp#12296

Signed-off-by: Brad Fitzpatrick <[email protected]>
  • Loading branch information
bradfitz committed Jun 15, 2023
1 parent f5464dd commit dd4570e
Show file tree
Hide file tree
Showing 2 changed files with 5 additions and 89 deletions.
10 changes: 5 additions & 5 deletions http2/transport.go
Original file line number Diff line number Diff line change
Expand Up @@ -1266,11 +1266,11 @@ func (cc *ClientConn) RoundTrip(req *http.Request) (*http.Response, error) {
return res, nil
}

cancelRequest := func(cs *clientStream, err error) error {
cancelRequest := func(cs *clientStream, markDoNotReuse bool, err error) error {
cs.cc.mu.Lock()
cs.abortStreamLocked(err)
bodyClosed := cs.reqBodyClosed
if cs.ID != 0 {
if markDoNotReuse && cs.ID != 0 {
// This request may have failed because of a problem with the connection,
// or for some unrelated reason. (For example, the user might have canceled
// the request without waiting for a response.) Mark the connection as
Expand Down Expand Up @@ -1318,12 +1318,12 @@ func (cc *ClientConn) RoundTrip(req *http.Request) (*http.Response, error) {
return handleResponseHeaders()
default:
waitDone()
return nil, cancelRequest(cs, cs.abortErr)
return nil, cancelRequest(cs, true, cs.abortErr)
}
case <-ctx.Done():
return nil, cancelRequest(cs, ctx.Err())
return nil, cancelRequest(cs, false, ctx.Err())
case <-cs.reqCancel:
return nil, cancelRequest(cs, errRequestCanceled)
return nil, cancelRequest(cs, false, errRequestCanceled)
}
}
}
Expand Down
84 changes: 0 additions & 84 deletions http2/transport_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6374,87 +6374,3 @@ func (c *blockReadConn) Read(b []byte) (n int, err error) {
<-c.blockc
return c.Conn.Read(b)
}

func TestTransportReuseAfterError(t *testing.T) {
serverReqc := make(chan struct{}, 3)
st := newServerTester(t, func(w http.ResponseWriter, r *http.Request) {
serverReqc <- struct{}{}
}, optOnlyServer)
defer st.Close()

var (
unblockOnce sync.Once
blockc = make(chan struct{})
connCountMu sync.Mutex
connCount int
)
tr := &Transport{
TLSClientConfig: tlsConfigInsecure,
DialTLS: func(network, addr string, cfg *tls.Config) (net.Conn, error) {
// The first connection dialed will block on reads until blockc is closed.
connCountMu.Lock()
defer connCountMu.Unlock()
connCount++
conn, err := tls.Dial(network, addr, cfg)
if err != nil {
return nil, err
}
if connCount == 1 {
return &blockReadConn{
Conn: conn,
blockc: blockc,
}, nil
}
return conn, nil
},
}
defer tr.CloseIdleConnections()
defer unblockOnce.Do(func() {
// Ensure that reads on blockc are unblocked if we return early.
close(blockc)
})

req, _ := http.NewRequest("GET", st.ts.URL, nil)

// Request 1 is made on conn 1.
// Reading the response will block.
// Wait until the server receives the request, and continue.
req1c := make(chan struct{})
go func() {
defer close(req1c)
res1, err := tr.RoundTrip(req.Clone(context.Background()))
if err != nil {
t.Errorf("request 1: %v", err)
} else {
res1.Body.Close()
}
}()
<-serverReqc

// Request 2 is also made on conn 1.
// Reading the response will block.
// The request is canceled once the server receives it.
// Conn 1 should now be flagged as unfit for reuse.
req2Ctx, cancel := context.WithCancel(context.Background())
go func() {
<-serverReqc
cancel()
}()
_, err := tr.RoundTrip(req.Clone(req2Ctx))
if err == nil {
t.Errorf("request 2 unexpectedly succeeded (want cancel)")
}

// Request 3 is made on a new conn, and succeeds.
res3, err := tr.RoundTrip(req.Clone(context.Background()))
if err != nil {
t.Fatalf("request 3: %v", err)
}
res3.Body.Close()

// Unblock conn 1, and verify that request 1 completes.
unblockOnce.Do(func() {
close(blockc)
})
<-req1c
}

0 comments on commit dd4570e

Please sign in to comment.