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

StreamWriter expects to be called on some internal EventLoop #200

Closed
weissi opened this issue Apr 5, 2020 · 5 comments · Fixed by #215
Closed

StreamWriter expects to be called on some internal EventLoop #200

weissi opened this issue Apr 5, 2020 · 5 comments · Fixed by #215
Assignees
Labels
kind/bug Feature doesn't work as expected.
Milestone

Comments

@weissi
Copy link
Contributor

weissi commented Apr 5, 2020

The HTTPClient.Body.StreamWriter that you get when using the default .stream(length: contentLength, { thisStreamWriterHere in }) expects to be called on the internal EventLoop that belongs to the underlying Channel. The user however doesn't get access to that Channel at all so it's impossible to use this API :(.

This needs a fix really soon.

@weissi weissi added the kind/bug Feature doesn't work as expected. label Apr 5, 2020
@artemredkin artemredkin added this to the 1.2.0 milestone Apr 26, 2020
@weissi
Copy link
Contributor Author

weissi commented May 14, 2020

After a chat with @ktoso, I noticed that this ticket isn't clear enough. The rules should be:

  • All callouts (delegate, futures, callbacks, ...) AHC makes to the user should be on the preferred EL, no matter where the Channel lives
  • All callins (returned futures, callins, ...) from the user must be tolerated on any thread.

IMHO fixing this issue is blocking 1.2.0, we cannot possibly release it as on master as it's impossible to use right now.

@ktoso
Copy link
Contributor

ktoso commented May 15, 2020

I started looking into this a bit btw, may have more time soon to dedicate to this.

@artemredkin
Copy link
Collaborator

artemredkin commented May 18, 2020

It seems that this is more about API design, than a programming model, no? Even if I fix #192 in #215 , it will still not solve this issue if I don't want to specify task EL myself, right?

@weissi
Copy link
Contributor Author

weissi commented May 19, 2020

@artemredkin Not sure, the write API for StreamWriter can stay as is but it can't make the assumption it makes today because that makes it impossible to use with the connection pool.
In theory, we could mandate that the user calls us back on the delegate's EL but that would be of no benefit to us because we need to hop EL's to the Channel's EL anyway so we should just accept callins on any EL.

@weissi
Copy link
Contributor Author

weissi commented May 19, 2020

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Feature doesn't work as expected.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants