Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
edaniels opened this issue Dec 17, 2022 · 2 comments
Open

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

edaniels opened this issue Dec 17, 2022 · 2 comments
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone

Comments

@edaniels
Copy link
Contributor

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).

@gopherbot gopherbot added this to the Unreleased milestone Dec 17, 2022
@edaniels
Copy link
Contributor Author

This was further complicated by us never seeing this issue in practice until golang/net@a870f35 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
Copy link
Contributor

@neild can you give this a look?

@dr2chase dr2chase added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Dec 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Development

No branches or pull requests

3 participants