Skip to content

Commit 943fd67

Browse files
committed
http2: fix off-by-one error in client check for max concurrent streams
Fixes golang/go#48358. Change-Id: Ib4eb93702b32ae7d03cad17ca0b997d5e6a58ad7 Reviewed-on: https://go-review.googlesource.com/c/net/+/349490 Trust: Damien Neil <[email protected]> Run-TryBot: Damien Neil <[email protected]> TryBot-Result: Go Bot <[email protected]> Reviewed-by: Brad Fitzpatrick <[email protected]>
1 parent 7bf0baf commit 943fd67

File tree

2 files changed

+47
-2
lines changed

2 files changed

+47
-2
lines changed

http2/transport.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -800,7 +800,7 @@ func (cc *ClientConn) idleStateLocked() (st clientConnIdleState) {
800800
// writing it.
801801
maxConcurrentOkay = true
802802
} else {
803-
maxConcurrentOkay = int64(len(cc.streams)+1) < int64(cc.maxConcurrentStreams)
803+
maxConcurrentOkay = int64(len(cc.streams)+1) <= int64(cc.maxConcurrentStreams)
804804
}
805805

806806
st.canTakeNewRequest = cc.goAway == nil && !cc.closed && !cc.closing && maxConcurrentOkay &&
@@ -1207,7 +1207,7 @@ func (cc *ClientConn) awaitOpenSlotForRequest(req *http.Request) error {
12071207
return errClientConnUnusable
12081208
}
12091209
cc.lastIdle = time.Time{}
1210-
if int64(len(cc.streams))+1 <= int64(cc.maxConcurrentStreams) {
1210+
if int64(len(cc.streams)) < int64(cc.maxConcurrentStreams) {
12111211
if waitingForConn != nil {
12121212
close(waitingForConn)
12131213
}

http2/transport_test.go

+45
Original file line numberDiff line numberDiff line change
@@ -3824,6 +3824,51 @@ func TestTransportResponseDataBeforeHeaders(t *testing.T) {
38243824
ct.run()
38253825
}
38263826

3827+
func TestTransportRequestsLowServerLimit(t *testing.T) {
3828+
st := newServerTester(t, func(w http.ResponseWriter, r *http.Request) {
3829+
}, optOnlyServer, func(s *Server) {
3830+
s.MaxConcurrentStreams = 1
3831+
})
3832+
defer st.Close()
3833+
3834+
var (
3835+
connCountMu sync.Mutex
3836+
connCount int
3837+
)
3838+
tr := &Transport{
3839+
TLSClientConfig: tlsConfigInsecure,
3840+
DialTLS: func(network, addr string, cfg *tls.Config) (net.Conn, error) {
3841+
connCountMu.Lock()
3842+
defer connCountMu.Unlock()
3843+
connCount++
3844+
return tls.Dial(network, addr, cfg)
3845+
},
3846+
}
3847+
defer tr.CloseIdleConnections()
3848+
3849+
const reqCount = 3
3850+
for i := 0; i < reqCount; i++ {
3851+
req, err := http.NewRequest("GET", st.ts.URL, nil)
3852+
if err != nil {
3853+
t.Fatal(err)
3854+
}
3855+
res, err := tr.RoundTrip(req)
3856+
if err != nil {
3857+
t.Fatal(err)
3858+
}
3859+
if got, want := res.StatusCode, 200; got != want {
3860+
t.Errorf("StatusCode = %v; want %v", got, want)
3861+
}
3862+
if res != nil && res.Body != nil {
3863+
res.Body.Close()
3864+
}
3865+
}
3866+
3867+
if connCount != 1 {
3868+
t.Errorf("created %v connections for %v requests, want 1", connCount, reqCount)
3869+
}
3870+
}
3871+
38273872
// tests Transport.StrictMaxConcurrentStreams
38283873
func TestTransportRequestsStallAtServerLimit(t *testing.T) {
38293874
const maxConcurrent = 2

0 commit comments

Comments
 (0)