-
Notifications
You must be signed in to change notification settings - Fork 22
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
Properly close streams #104
Properly close streams #104
Conversation
Incidentally, without this PR, a call to |
We still depend on a fix to `http2`; without it, depending on whether or not the GC is able to detect an STM deadlock, the tests can hang. See kazu-yamamoto/http2#104 for details.
We still depend on a fix to `http2`; without it, depending on whether or not the GC is able to detect an STM deadlock, the tests can hang. See kazu-yamamoto/http2#104 for details.
We still depend on a fix to `http2`; without it, depending on whether or not the GC is able to detect an STM deadlock, the tests can hang. See kazu-yamamoto/http2#104 for details.
@kazu-yamamoto FWIW the aforementioned @edsko's blogpost has been published at https://well-typed.com/blog/2024/01/when-blocked-indefinitely-is-not-indefinite/ |
Yes. I tweeted about the blog post already. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Rebased and merged. |
This avoids another "blocked indefinitely" in client code, following on from #97. I suspect that there are probably others, still.
@kazu-yamamoto I'm not entirely sure which exception to use when the stream is closed; if
ConnectionIsClosed
is not the right one, let me know. Also, I think the firstOpen
/Open
case probably never arises in practice, but this way the code doesn't make any assumptions; but in principle I think we can just leave out this case (but I'm not 100% sure).Incidentally, I am writing a blog post about deadlock detection in STM, which also illustrates why fixes like this are important; I'll post a link once I have a preview in case you're curious.