golang / go

The Go programming language
https://go.dev
BSD 3-Clause "New" or "Revised" License
123.68k stars 17.62k forks source link

x/net: http2 server onReadTimeout does not cancel context #57359

Open edaniels opened 1 year ago

edaniels commented 1 year ago

What version of Go are you using (go version)?

1.19.3

Does this issue reproduce with the latest release?

Yes

What operating system and processor architecture are you using (go env)?

GOARCH="arm64" GOOS="darwin"

What did you do?

We have a grpc bidi streaming handler that we serve via the server's experimental ServeHTTP method. We've found that when a server streaming request comes in, that does not send its first request body longer than the http server's ReadTimeout, that the context is not closed.

What did you expect to see?

For the context to be closed.

What did you see instead?

That operations using that context block forever until we do a read on the body.

I believe this hasn't really come up in practice because it's quite odd to do anything with a request context prior to reading the body. However, in this bidirectional call, it's actually the server that sends the first message (response) and then the client sends the second message in response to the server (a flipping of the client server relationship if you will). We can certainly work around this but it took a very long time to debug why we never saw any cancelation/timeouts happen until our first server write happened that ultimately causes a client side write and then a server side read (which is long dead).

edaniels commented 1 year ago

This was further complicated by us never seeing this issue in practice until https://github.com/golang/net/commit/a870f3529a284d7f9d1d3e0c805430d14d407727 was committed which started to break things. Prior to that point, we could read freely, even if we were wrong about that assumption. It would seem better that in addition to closing the response body reader, that the request context also closes since this context may be used in other places during the request and functions similarly for timing out / canceling the request all together if a body has not been read.

dr2chase commented 1 year ago

@neild can you give this a look?