The context tests were flaky under Go 1.6 on Windows. Make them
slower, but make up for the slowness by parallelizing them.
These tests don't run on Go 1.7+ in the subrepo. They were deflaked in
the standard library's context in Go 1.7.
Updates golang/go#11811
Change-Id: I8dc8d9e13e72e87b4444e92d2316dd95bd7d066d
Reviewed-on: https://go-review.googlesource.com/34288
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
resetStream(st) queues a RST_STREAM frame and then calls closeStream(st).
Unfortunately, closeStream(st) flushes any writes pending on st, which
can drop the RST_STREAM that was just queued. (If we are lucky, the
RST_STREAM will fit in the send buffer and won't be dropped, however,
if we are unlucky the RST_STREAM will be dropped.)
I fixed this bug by removing closeStream(st) from resetStream. Instead,
closeStream happens after the RST_STREAM frame is written. This is a more
direct implementation of the diagram in RFC 7540 Section 5.1, which says
that a stream does not transition to "closed" until after the RST_STREAM
has been sent.
A side-effect is that the stream may stay open for longer than it did
previously (since it won't close until *after* the RST_STREAM frame is
actually written). Situations like the following are a problem:
- Client sends a DATA frame that exceeds its flow-control window
- Server returns streamError(ErrCodeFlowControl) from processData
- RST_STREAM is queued behind other frames
- Server process the request body and releases flow-control
- Client sends another DATA frame, this one fits in the flow-control
window. Server should NOT process this frame.
To avoid the above problem, we set a bool st.resetQueued=true when
RST_STREAM is queued, then ignore all future incoming HEADERS and DATA
frames on that stream.
I also removed st.sentReset and st.gotReset, which were used to ignore
frames after RST_STREAM is sent. Now we just check if the stream is closed.
Fixesgolang/go#18111
Change-Id: Ieb7c848989431add5b7d95f40d6d91db7edc4980
Reviewed-on: https://go-review.googlesource.com/34238
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
To help debug rare nondeterministic server_test failures, log Framer
reads and writes. Sample output from an injected test failure:
$ go test golang.org/x/net/http2
--- FAIL: TestServer_Request_Reject_Pseudo_scheme_invalid (0.00s)
server_test.go:998: server request made it to handler; should've been rejected
server_test.go:514: got a *http2.HeadersFrame; want *RSTStreamFrame
server_test.go:229: Framer read log:
2016-12-07 17:06:11.907199013 Framer 0xc4212665b0: read SETTINGS len=18, settings: MAX_FRAME_SIZE=1048576, MAX_CONCURRENT_STREAMS=250, MAX_HEADER_LIST_SIZE=1048896
2016-12-07 17:06:11.907523124 Framer 0xc4212665b0: read SETTINGS flags=ACK len=0
2016-12-07 17:06:11.908090453 Framer 0xc4212665b0: read HEADERS flags=END_STREAM|END_HEADERS stream=1 len=48
server_test.go:235: Framer write log:
2016-12-07 17:06:11.907152927 Framer 0xc4212665b0: wrote SETTINGS len=0
2016-12-07 17:06:11.907207016 Framer 0xc4212665b0: wrote SETTINGS flags=ACK len=0
2016-12-07 17:06:11.907550525 Framer 0xc4212665b0: wrote HEADERS flags=END_STREAM|END_HEADERS stream=1 len=16
Framer logs are written at the end of a test only if the test failed
and only if http2debug!=2. (If http2debug=2, then Framer logs are already
written to stdout.)
Hopefully this will help debug flaky tests. Or it might not.
The code is kind of ugly.
Updates golang/go#18235
Change-Id: I74c8ef82521d81f123663c98c883c71d9843964f
Reviewed-on: https://go-review.googlesource.com/34130
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
TestConnUnicastSocketOptions doesn't work on nacl, plan9, or windows,
but the t.Skip at the top was missing the GOOS == "windows" check. (It
was present in the ipv6 version of the test)
The test than proceeded to try to run and set up a connection pair
for testing. Here's what was happening.
Goroutines are M (main, the client) and S (server, doing the Accept):
M: net.Listen("tcp4", "127.0.0.1:0"), succeeds.
M: defer ln.Close() enqueued.
M: go acceptor() (start "S" (server) goroutine)
M: net.Dial("tcp4", ln.Addr().String()), succeeds.
Notably, it's connected to the kernel's TCP stack at this point.
The userspace Accept hasn't run. Go's listen backlog is
syscall.SOMAXCONN on Windows (which is fine).
M: testUnicastSocketOptions:
- finds that it's GOOS == "windows",
- calls t.Skipf, which calls runtime.Goexit, runs deferred goroutines
M: ln.Close() (previously deferred runs)
M: test completes
Then:
S: server goroutine finally schedules
S: c, err := ln.Accept(), gets an error (because it's closed)
if err != nil {
t.Error(err) // setting an error after test is done; bogus
So, fix it both ways: skip the test earlier on "windows", and simplify
the server goroutine and move the failing into the main goroutine.
Also skip other tests earlier on Windows that will also t.Skip. They
don't have goroutines, but might as well skip earlier.
Fixesgolang/go#17655
Change-Id: I5d910d70943be37b2b4b56542ee98e42fc3acd71
Reviewed-on: https://go-review.googlesource.com/34021
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Bryan Mills <bcmills@google.com>
The interface for net.Conn is getting stricter for the Go 1.8 release.
We add a package that tests that a given net.Conn implementation
properly satisfies the interface.
Fixesgolang/go#18097
Change-Id: I5e9f1f3c7cb5c060d734ed7e3a24886d4213c4e1
Reviewed-on: https://go-review.googlesource.com/33679
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
https://go-review.googlesource.com/33238 fixed scheduling of RST_STREAMs so
they are added to the appropriate stream queue. However, RST_STREAM may be
sent before OpenStream or after CloseStream in certain error cases, such as:
00ed5e97ea/http2/server.go (L1395)00ed5e97ea/http2/frame.go (L866)00ed5e97ea/http2/frame.go (L947)
In these cases, the failing stream has no queue and in priorityWriteScheduler.Push
will panic. A simple fix is to add RST_STREAMs to the root queue when the stream
queue doesn't exist. This is correct because:
- RST_STREAM is tiny and doesn't use flow control bytes, so prioritization is
not important.
- The server should not send any frames on a stream after sending RST_STREAM,
but even if that happens, the RST_STREAM will be ordered before those other
frames because the control queue has the highest priority.
Fixesgolang/go#17919
Change-Id: I2e8101f015822ef975303a1fe87634b36afbdc6b
Reviewed-on: https://go-review.googlesource.com/33248
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
If the user is using Server.SetKeepAlivesEnabled(false), assume they
have a reason and respect it. Make HTTP/2 behave like HTTP/1 (except a
bit nicer, since we have GOAWAY).
Updates golang/go#17717
Change-Id: I4a5ce324f0ac8653d83e75029063cd2e270a1048
Reviewed-on: https://go-review.googlesource.com/33153
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
The max allowed stream ID is 2^31-1. The RFC says that we should send
GOAWAY if the ID space is exhausted.
Change-Id: I0042cb4e1f8781a2ece5a5f06f498eb06192da7f
Reviewed-on: https://go-review.googlesource.com/32488
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Go policy has been single space after periods in comments for some time.
The copyright header template at:
https://golang.org/doc/contribute.html#copyright
also uses a single space.
Make them all consistent.
This CL was generated with:
perl -i -npe 's,^(// Copyright [0-9]+ The Go Authors\.) (All rights reserved\.)$,$1 $2,' $(git grep -l -E '^// Copyright [0-9]+ The Go Authors\. All rights reserved\.$')
Follows https://golang.org/cl/20111.
Change-Id: I66671dddf821f5dc027bc254e0196b3e3a2bdf3b
Reviewed-on: https://go-review.googlesource.com/32878
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Fix sc.state, which was returning "idle" instead of "closed" if the max
pushed stream ID happened to exceed the max client stream ID. This caused
us to erroneously generate connection errors because we believed a state
invariant had been violated when it had not.
I also renamed maxStreamID to maxClientStreamID for clarity, since we
need to track client-generated and server-generated stream IDs separately.
Fixesgolang/go#17777
Change-Id: Id3d5700d79cc699a267bd91d6ebace0770fa62fc
Reviewed-on: https://go-review.googlesource.com/32755
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
https://golang.org/cl/31727 made many of the Server Frame processors
ignore incoming frames if the connection was in shutdown mode.
The idea was that it's pointless to do work if we're about to hang up
on them in 250ms anyway for violating a protocol error.
But as of https://golang.org/cl/32412 (graceful shutdown) we can also
be in "go away" mode for ErrCodeNo, which just means to nicely tell
them to GOAWAY and because they did nothing wrong, we don't hang up in
250ms (the value of which gave the peer time to read the error before
the TCP close), but instead we keep the conn open until it's idle.
The combination of the two CLs made TestTransportAndServerSharedBodyRace_h2
flaky, since it was never seeing RST_STREAM on cancelation of requests,
and then Handlers would block forever.
Updates golang/go#17733 (fixes after bundle into std)
Change-Id: I67491c1d7124bc3cb554f9246ea7683f20b6a4e3
Reviewed-on: https://go-review.googlesource.com/32583
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Crawshaw <crawshaw@golang.org>
If the client canceled a request, the Transport would then send a
RST_STREAM, but also would close the Response.Body's pipe. Meanwhile,
the server's DATA response could already be on the wire in
flight. We'd then read it, attempt to write its bytes to the buffer,
find it already closed, bubble up that error, and ultimately close the
whole TCP connection (breaking all other open streams).
So, don't do that.
Keep track of which connections we've sent RST_STREAM to, and ignore
DATA frames on those streams.
Updates golang/go#16974 (fixes after bundle to std)
Change-Id: Ic29a3aefff5241146cd2ca80aafa35fc4fb18b6e
Reviewed-on: https://go-review.googlesource.com/32571
Reviewed-by: Tom Bergan <tombergan@google.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
"h2-14" was draft 14 of http2. It's past time to retire announcing it.
Everybody has been using "h2" in the wild for quite some time now.
The Google GFE doesn't accept it anymore either.
Change-Id: I087681a0a5e7117de3ab74a2f3f9e0ae45e007cd
Reviewed-on: https://go-review.googlesource.com/32576
Reviewed-by: Tom Bergan <tombergan@google.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Currently there is no way to pass request scoped information from
the handler to the FileSytem interface. This can be important
to pass credentials or timeout parameters to the FileSystem
operations. Pipe context through the request from
http.Request.Context(). With pre-go1.7 use context.Background().
Custom FileSystem implementations will need to change, but it will
only require a new argument in each of the FileSystem methods.
The change will be trivial to update to.
Fixesgolang/go#17658
Change-Id: I7491faf3467ad55db793a68081e074a9b3f9624d
Reviewed-on: https://go-review.googlesource.com/32421
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
The previous way was causing init blocks to be generated which the
linker wasn't smart enough to discard, and the cmd/go tool was once
again including http1 & http2 Servers.
Change-Id: I9d82e14421e0faa96437668f9013d1174f009936
Reviewed-on: https://go-review.googlesource.com/32417
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Daniel Theophanes <kardianos@gmail.com>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This adds support for gracefully shutting down the Server, including
sending a GOAWAY frame to clients and shutting down when things are
idle. For now this support is only when integrated with the standard
library's Server.
A future change could export some of this.
Updates golang/go#4674
Change-Id: I78cd4f58ca529bf9d149054f929d9089e7685875
Reviewed-on: https://go-review.googlesource.com/32412
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
The net/http package is now better at HTTP redirects, but that broke
these tests which were trying to test the WebDAV server handler's
behavior for a single roundtrip but were accidentally using the HTTP
client (which does redirects) instead.
Use the http.Transport which only does a single HTTP roundtrip.
Change-Id: I8a0cfe2f841effc2f50c26f90c140e94e256a0ac
Reviewed-on: https://go-review.googlesource.com/32413
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Dominik Honnef <dominik@honnef.co>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
The console package empirically seems to require \r\n line endings now,
else the print head does not go back to the left of the screen.
Change-Id: I49f1870b8c48da254afaeaf3daf2e8fdd0432d52
Reviewed-on: https://go-review.googlesource.com/32321
Reviewed-by: Blake Mizerany <blake.mizerany@gmail.com>
CancelFunc was the only part of the context package which didn't
forward nicely with the move from x/net/context to std context.
Use it for Context as well.
Change-Id: Ieff39b10b0783d55d0437c73923053297ed0ea4a
Reviewed-on: https://go-review.googlesource.com/32317
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Rob Pike <r@golang.org>
This makes ConfigureServer initialize the http2 Server's IdleTimeout
from the http1 Server configuration, using the same rules as
IdleTimeout in Go 1.8: first use IdleTimeout if specified, else fall
back to the old ReadTimeout.
Updates golang/go#14204
Change-Id: I4dee971f8416ef0cbf99335a199c46355f9ab09d
Reviewed-on: https://go-review.googlesource.com/32230
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Tom Bergan <tombergan@google.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
