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.

Sign up for GitHub

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

Jump to bottom

StapledStream.send_eof should error out if you call it while send_all is pending #1180

Open
njsmith opened this issue Aug 9, 2019 · 0 comments · May be fixed by #1181
Open

StapledStream.send_eof should error out if you call it while send_all is pending #1180

njsmith opened this issue Aug 9, 2019 · 0 comments · May be fixed by #1181
Labels
communication

Comments

@njsmith
Copy link
Member

njsmith commented Aug 9, 2019

Noticed while working on #823: normally, if you try to call send_eof on an object at the same time that another task is already calling send_all or wait_send_all_might_not_block, then send_eof should raise BusyResourceError. But StapledStream.send_eof calls self.send_stream.aclose, which has a different semantics: it interrupts any pending calls. So right now StapledStream.send_eof has the wrong semantics.

To solve this, StapledStream should detect when another task is already calling a send method, and in that case make send_eof raise an exception instead of calling self.send_stream.aclose.

I have a fix for this in my #823 branch, that I'll post shortly.
njsmith added a commit to njsmith/trio that referenced this issue Aug 9, 2019
@njsmith
Make send_eof a standard part of the Stream interface
e5c67da 
The only reason HalfCloseableStream existed as a separate interface
was because SSLStream couldn't support send_eof. But TLS 1.3 made it
possible to support send_eof! So now the static split doesn't make
sense. Instead, we move send_eof into the Stream interface (though it
can still fail at runtime with NotImplementedError), and get rid of
HalfCloseableStream. (Fixes: python-triogh-823.)

This also includes a fix for StapledStream.send_eof, that was revealed
by the new enhanced check_two_way_stream. (Fixes: python-triogh-1180).
@njsmith njsmith linked a pull request Aug 9, 2019 that will close this issue
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
communication
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Make send_eof a standard part of the Stream interface njsmith/trio
2 participants
@njsmith @oremanj