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

unset yield on flush #44

Merged
merged 1 commit into from
Jul 18, 2018
Merged

Conversation

dpatti
Copy link
Contributor

@dpatti dpatti commented Jul 16, 2018

If you ask a serializer to yield, then synchronously use it in such a
way that you no longer want it to yield, there is no way to "undo" the
yield aside from using operation twice and ignoring the first yield.
The semantics of flush should be that it unsets any possible yield
flag so that the next operation has a chance to read pending iovecs.

I did not include tests, particularly because this was difficult to
encapsulate with the given framework. We need to be able to inspect the
state of the serializer without closing it, and all current serialize
functions do close it in order to dump the contents to a string or
bigstring.

If you ask a serializer to `yield`, then synchronously use it in such a
way that you no longer want it to yield, there is no way to "undo" the
yield aside from using `operation` twice and ignoring the first yield.
The semantics of `flush` should be that it unsets any possible `yield`
flag so that the next operation has a chance to read pending iovecs.

I did not include tests, particularly because this was difficult to
encapsulate with the given framework. We need to be able to inspect the
state of the serializer without closing it, and all current serialize
functions do close it in order to dump the contents to a string or
bigstring.
dpatti added a commit to dpatti/httpaf that referenced this pull request Jul 16, 2018
This test exposes an issue where a response hangs indefinitely because
`respond_with_streaming` puts the writer into `yield` mode so that it
doesn't write until the first `flush`, but if the first `flush` call
happens before the write loop "exhausts" the yield, it will just get
stuck.

This requires a change in Faraday [1] to fix.

[1]: inhabitedtype/faraday#44
@seliopou seliopou merged commit 7994a16 into inhabitedtype:master Jul 18, 2018
@dpatti dpatti deleted the flush-unsets-yield branch July 27, 2018 16:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants