AFAICT, activeRes serves no real purpose. It is used in just two ways:
- To reduce the number of calls to closeIfIdle, which reduces the number
of acquires of cc.mu when there are many concurrent streams. I dug
through the CL history and could not find any benchmarks showing that
this is necessary.
- To avoid redundant calls to cs.bufPipe.CloseWithError(err) when a read
loop is shutdown. This is unnecessary, since redundant CloseWithError
calls are ignored.
Since there isn't a good reason to have activeRes, the simplest way to
fix the leak is to remove activeRes entirely.
Updates golang/go#21543
Change-Id: I1d1d2dc6c946425a2772c8bf71436707021ac269
Reviewed-on: https://go-review.googlesource.com/80137
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This fixes TestTrailersClientToServer_h2. Before this CL, the following
command reliably fails. With this CL merged into net/http, the following
command reliably succeeds.
go test -race -run=TestTrailersClientToServer_h2 -count 1000 net/http
Updates golang/go#22721
Change-Id: I05d1504c60854fcf3ae9531f36a126e94b00f0b7
Reviewed-on: https://go-review.googlesource.com/79238
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Emmanuel Odeke <emm.odeke@gmail.com>
In golang/go#22880, our http2 server was sending a HEADERS response
without a :status header. Our client code correctly returned an error
from RoundTrip, but we forgot to clean up properly, and then
subsequently crashed on a DATA frame.
This fixes the Transport crash. A fix for the server bug will come separately.
Change-Id: Iea3bcf4a8c95963c8b5e2b6dd722177607bd1bc1
Reviewed-on: https://go-review.googlesource.com/80056
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Tom Bergan <tombergan@google.com>
The HTTP/2 RFC does indeed mandate TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256
but in practice, people are also using TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256
becuase they are only using an ECDSA certificate. This is the case in acme/autocert.
It doesn't make sense to enforce only RSA in cipher suites if it will
never be used because they are using a ECDSA certificate.
Change-Id: I86dac192a3eb9b74e4268310a3b550b3bd88a37f
Reviewed-on: https://go-review.googlesource.com/30721
Reviewed-by: Tom Bergan <tombergan@google.com>
Run-TryBot: Tom Bergan <tombergan@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Based on golang/go#19653, it should be possible to reuse an http.Request
object after the outstanding request has completed. This CL fixes a race
in the http/2 library that occurs when a caller tries to reuse an
http.Request just after the request completed.
The new test failed with -race before this CL and passes after this CL.
Verified with -count 10000.
Updates golang/go#21316
Change-Id: I014cf9cefd0dd21f6f41763ba554d23ddc7fca40
Reviewed-on: https://go-review.googlesource.com/75530
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
There was a case where we forgot to undo this wrapper. Instead of fixing
that case, I moved the implementation of ClientConn.RoundTrip into an
unexported method that returns the same info as a bool.
Fixesgolang/go#22136
Change-Id: I7e5fc467f9c26fb74b9b83f2b3b7f8882645e34c
Reviewed-on: https://go-review.googlesource.com/75252
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Add an AppendPack method to Message that appends the message data into a
byte buffer. Reusing a buffer allows for a reduction in allocations.
name time/op
Pack-8 5.04µs ± 1%
AppendPack-8 4.95µs ± 2%
name alloc/op
Pack-8 6.22kB ± 0%
AppendPack-8 5.71kB ± 0%
name allocs/op
Pack-8 21.0 ± 0%
AppendPack-8 20.0 ± 0%
Change-Id: I8bb6b07787cf2ba9ef32e1e60a3003a585ec55be
Reviewed-on: https://go-review.googlesource.com/45274
Reviewed-by: Ian Gudger <igudger@google.com>
Reviewed-by: Mikio Hara <mikioh.mikioh@gmail.com>
Run-TryBot: Mikio Hara <mikioh.mikioh@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
The macOS kernel reliably crashes in a repeated TestRouteMessage.
Putting some extra padding into the request buffer avoids the crash.
This will do as workaround; the kernel bug will be reported to Apple separately.
Fixesgolang/go#22456.
Change-Id: I789d3d57fbc511016d9f4a3fa7662d6c7642f137
Reviewed-on: https://go-review.googlesource.com/73690
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Currently, we close the connection immediately after sending a GOAWAY
frame if all outstanding responses have been completely sent. However,
the client may have had requests in-flight at that time which have been
queued in the kernel receive buffer. On both Windows and Linux, if the
connection is close()'d when the receive buffer is not empty, the kernel
sends RST. This has the effect of aborting both sides of the connection,
meaning the client may not actually receive all responses that were sent
before the GOAWAY.
Instead, we should delay calling close() until after the receive buffer
has been drained. We don't want to delay indefinitely, which means we
need some kind of timeout. Ideally that timeout should be about 1 RTT +
epsilon, under the assumption that the client will not send any more
frames after receiving the GOAWAY. However, 1 RTT is difficult to
measure. It turns out we were already using a 1 second delay in other
cases, so we reuse that same delay here.
Note that we do not call CloseWrite() to half-close the underlying TLS
connection. This seems unnecessary -- GOAWAY is effectively a half-close
at the HTTP/2 level.
Updates golang/go#18701 (fixes after it's bundled into net/http)
Change-Id: I4d68bada6369ba95e5db02afe6dfad0a393c0334
Reviewed-on: https://go-review.googlesource.com/71372
Run-TryBot: Tom Bergan <tombergan@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
When a client receives a HEADER frame with a END_STREAM flag,
clientConn.streamByID closes the stream before processing the headers
which may contain a full non-error response. This causes the request's
bodyWriter cancelation to race with the response.
Closing the stream after processing headers allows the response to be
available before the bodyWriter is canceled.
Updates golang/go#20521
Change-Id: I70740e88f75240836e922163a54a6cd89535f7b3
Reviewed-on: https://go-review.googlesource.com/70510
Run-TryBot: Emmanuel Odeke <emm.odeke@gmail.com>
Reviewed-by: Tom Bergan <tombergan@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Move the README to README.md so Gerrit can render it; currently
Gerrit only renders files named exactly "README.md" (for example at
https://go.googlesource.com/go).
Add more links to the README explaining how to file issues,
how to submit code changes, where to download the code to and
how to get it. Hopefully this should help people who go to
https://go.googlesource.com/net or https://github.com/golang/net
figure out how to get started with development.
Change-Id: I4eb54f2b210e4f028c9f00955ff19b55643315e6
Reviewed-on: https://go-review.googlesource.com/49850
Reviewed-by: Mikio Hara <mikioh.mikioh@gmail.com>
Some elements and attributes have been removed from the spec,
but are kept here for backwards compatibility.
Also, this makes gen.go support go:generate and write contents into
target files directly.
Finally, this makes `go generate` work on an entire directory.
Change-Id: I8d41840eec69eec1ef08527d8d71786612a3121d
Reviewed-on: https://go-review.googlesource.com/65152
Run-TryBot: Tom Bergan <tombergan@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Tom Bergan <tombergan@google.com>
The number of children (494) got pretty close to the available maximum
number of children (511) in the table. This number increased over the
last year by roughly 65 indicating that we would run out of space in
the next three months. The other fields still have enough room left.
The following table show the growth over the last 1.5 years:
# Commit Date Children TextOffset Length High Low
b05061f 2017-09-18 494 28750 36 8407 8402
859d1a8 2017-09-06 494 28750 36 8407 8402
ddf80d0 2017-06-14 479 28411 36 8262 8257
61557ac 2017-01-26 466 28023 36 8121 8120
5695785 2016-10-20 434 27930 36 8135 8134
07b5174 2016-08-11 424 27866 36 8062 8051
7864c9e 2016-07-07 421 27811 36 8049 8038
3f122ce 2016-06-09 417 27680 36 8029 8018
d58ca66 2016-03-04 409 27059 36 7887 7886
6c581b9 2016-02-01 406 26999 36 7868 7867
78e1654 2016-01-20 405 26986 36 7863 7862
Given this rate of grow of max text offset it will overflow in 2021.
Thus use the last of the available 32 bits to encode more children.
Change-Id: I04db02100b202f220a0b4ee509f868db031fd8ab
Reviewed-on: https://go-review.googlesource.com/64330
Reviewed-by: Nigel Tao <nigeltao@golang.org>
Add a new peerMaxHeaderListSize member to ClientConn which records the
SETTINGS_MAX_HEADER_LIST_SIZE requested by the client's peer, and
respect this limit in (*ClientConn) encodeHeaders / encodeTrailers.
Attempting to send more than peerMaxHeaderListSize bytes of headers or
trailers will result in RoundTrip returning errRequestHeaderListSize.
Updates golang/go#13959
Change-Id: Ic707179782acdf8ae543429ea1af7f4f30e67e59
Reviewed-on: https://go-review.googlesource.com/29243
Run-TryBot: Tom Bergan <tombergan@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Tom Bergan <tombergan@google.com>
If the server returns a DATA frame before a HEADERS frame, the client
must close the connection with a PROTOCOL_ERROR as describe in the
section 8.1.2.6.
The current implementation does not reject such sequence and panics
trying to write on the non-initialized bufPipe.
Fixesgolang/go#21466
Change-Id: I55755dba259d026529a031e9ff82c915f5e4afae
Reviewed-on: https://go-review.googlesource.com/56770
Reviewed-by: Tom Bergan <tombergan@google.com>
Run-TryBot: Tom Bergan <tombergan@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Currently if the http2.Transport hits SettingsMaxConcurrentStreams for a
server, it just makes a new TCP connection and creates the stream on the
new connection. This CL updates that behavior to instead block RoundTrip
until a new stream is available.
I also fixed a second bug, which was necessary to make some tests pass:
Previously, a stream was removed from cc.streams only if either (a) we
received END_STREAM from the server, or (b) we received RST_STREAM from
the server. This CL removes a stream from cc.streams if the request was
cancelled (via ctx.Close, req.Cancel, or resp.Body.Close) before
receiving END_STREAM or RST_STREAM from the server.
Updates golang/go#13774
Updates golang/go#20985
Updates golang/go#21229
Change-Id: I660ffd724c4c513e0f1cc587b404bedb8aff80be
Reviewed-on: https://go-review.googlesource.com/53250
Run-TryBot: Tom Bergan <tombergan@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
We still support Go 1.6 for a few more days. (it'd normally be dropped
after Go 1.9 final is out)
And maybe we'll need to make a special case for supporting it longer
than normal if gRPC needs to.
Change-Id: I78675f1ef26aa09436a70d0f8aa3a0958768dd14
Reviewed-on: https://go-review.googlesource.com/53641
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
RoundTrip will retry a request if it receives REFUSED_STREAM. To guard
against servers that use REFUSED_STREAM to encourage rate limiting, or
servers that return REFUSED_STREAM deterministically for some requests,
we retry after an exponential backoff and we cap the number of retries.
The exponential backoff starts on the second retry, with a backoff
sequence of 1s, 2s, 4s, etc, with 10% random jitter.
The retry cap was set to 6, somewhat arbitrarily. Rationale: this is
what Firefox does.
Updates golang/go#20985
Change-Id: I4dcac4392ac4a3220d6d839f28bf943fe6b3fea7
Reviewed-on: https://go-review.googlesource.com/50471
Run-TryBot: Tom Bergan <tombergan@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Also makes it possible to capture RTM_DESYNC messages when you have a
routing protocol implementation that requires to be aware of the
health condition of kernel RIB on OpenBSD.
Change-Id: Idd0c8c5e8f5ea72a4d56c9a46c137786bcda6354
Reviewed-on: https://go-review.googlesource.com/50191
Run-TryBot: Mikio Hara <mikioh.mikioh@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Since Go 1.7 started including the context package in the core
distribution, this package has been forwarding to the core package's
implementation to the best of its ability. Since type aliases are due
to be included in the Go 1.9 release, use them to forward more
completely.
Change-Id: I97b92cd34b3f4426f565bb3d115f119d3c83a1dd
Reviewed-on: https://go-review.googlesource.com/49020
Run-TryBot: Emmanuel Odeke <emm.odeke@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
