فهرست منبع

http2: fix flake in net/http's TestCloseIdleConnections_h2

That test makes a request with no body and receives a response with no
body. The client will receive a HEADERS frame with END_STREAM. The test
assumes that the stream is closed immediately on receipt of that HEADERS
frame, i.e., before RoundTrip returns.

This assumption was broken by https://golang.org/cl/70510, which made
stream closure asynchronous w.r.t. RoundTrip.

To fix TestCloseIdleConnections_h2 while preserving the intent of CL
70510, we break processHeaders into two cases:

1. The request has a body. In this case, END_STREAM puts the stream in a
   half-closed-remote state, which means the connection is not
   necessarily idle when RoundTrip returns (since the request body is
   still being uploaded). In this case, we preserve the behavior from CL
   70510.

2. The request does not have a body. In this case, END_STREAM puts the
   stream in a closed state and we must close the stream before
   returning from RoundTrip.

The following command passes when this CL is merged into net/http:
go test -count=100000 -run=TestCloseIdleConnections_h2 net/http

Updates golang/go#22413

Change-Id: Iff2a0685a636ad51bff380e86a42b0d0eea984e5
Reviewed-on: https://go-review.googlesource.com/80139
Run-TryBot: Tom Bergan <tombergan@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Tom Bergan 8 سال پیش
والد
کامیت
894f8ed584
1فایلهای تغییر یافته به همراه16 افزوده شده و 8 حذف شده
  1. 16 8
      http2/transport.go

+ 16 - 8
http2/transport.go

@@ -1523,6 +1523,13 @@ func (rl *clientConnReadLoop) run() error {
 
 func (rl *clientConnReadLoop) processHeaders(f *MetaHeadersFrame) error {
 	cc := rl.cc
+	cs := cc.streamByID(f.StreamID, false)
+	if cs == nil {
+		// We'd get here if we canceled a request while the
+		// server had its response still in flight. So if this
+		// was just something we canceled, ignore it.
+		return nil
+	}
 	if f.StreamEnded() {
 		// Issue 20521: If the stream has ended, streamByID() causes
 		// clientStream.done to be closed, which causes the request's bodyWriter
@@ -1531,14 +1538,15 @@ func (rl *clientConnReadLoop) processHeaders(f *MetaHeadersFrame) error {
 		// Deferring stream closure allows the header processing to occur first.
 		// clientConn.RoundTrip may still receive the bodyWriter error first, but
 		// the fix for issue 16102 prioritises any response.
-		defer cc.streamByID(f.StreamID, true)
-	}
-	cs := cc.streamByID(f.StreamID, false)
-	if cs == nil {
-		// We'd get here if we canceled a request while the
-		// server had its response still in flight. So if this
-		// was just something we canceled, ignore it.
-		return nil
+		//
+		// Issue 22413: If there is no request body, we should close the
+		// stream before writing to cs.resc so that the stream is closed
+		// immediately once RoundTrip returns.
+		if cs.req.Body != nil {
+			defer cc.forgetStreamID(f.StreamID)
+		} else {
+			cc.forgetStreamID(f.StreamID)
+		}
 	}
 	if !cs.firstByte {
 		if cs.trace != nil {