-
Notifications
You must be signed in to change notification settings - Fork 9.2k
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
Don't reset previously closed stream, since it's racy. #7922
base: master
Are you sure you want to change the base?
Conversation
|
@yschimke I worked to the same solution as you for this bug and have been using it for 2 months without any issues.
I think the code returns the response object when the headers frame is received by okhttp. In the code above, try catch closes the response object, which closes the stream. The rfc says the following I think this change is perfect, I think chrome actually does something similar, where they dont send a RST_STREAM (Protocol_Error) for any data frames.
https://source.chromium.org/chromium/chromium/src/+/main:net/spdy/spdy_session.cc;l=2836 Another thing that should be looked into is maybe moving the code in this pr to the reset stream function for Http2.Writer, as there might be other cases where we send reset stream errors for streams we have already closed. |
To address #7913
We could look at the
nextStreamId
and cancel after an unexpected data frame. But arguably we should terminate the connection if we are getting unexpected frames.