Skip to content

client needs to tolerate futures from arbitrary EventLoops #95

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
Closed
weissi opened this issue Aug 31, 2019 · 1 comment · Fixed by #96
Closed

client needs to tolerate futures from arbitrary EventLoops #95

weissi opened this issue Aug 31, 2019 · 1 comment · Fixed by #96
Assignees
@artemredkin
Milestone
1.0.0

Comments

@weissi
Copy link
Contributor

weissi commented Aug 31, 2019

For back-pressure we've got a bunch of callbacks where the user can return futures to make the client continue. Those futures, at the moment, are required to be on the 'right' EventLoop but the client should definitely not require this. Instead, the client should always hop(to:) user futures to the correct EL if it requires it for a certain operation.

In most cases, the user will happen to return futures from the right EL anyway so hop(to:) is pretty much free (it fast paths eventLoop === future.eventLoop and just returns).
@weissi weissi added this to the 1.0.0 milestone Aug 31, 2019
@weissi
Copy link
Contributor Author

weissi commented Aug 31, 2019

example test cases for similar things (initialiser closures on the bootstraps in NIO): https://github.com/apple/swift-nio/blob/master/Tests/NIOTests/BootstrapTest.swift#L94-L209

http client should have similar test cases.

@artemredkin artemredkin self-assigned this Aug 31, 2019
@PopFlamingo PopFlamingo mentioned this issue Aug 31, 2019
Merged
@weissi weissi closed this as completed in #96 Sep 6, 2019
weissi pushed a commit that referenced this issue Sep 6, 2019
@weissi
Tolerate futures from arbitrary event loops (#96)
244aea6 
This commit fixes #95 by always hopping event loop futures received from
the delegate to the right event loop. This could be a source of bugs if
the library users forgot to hop(to:) futures from their delegates
implementations.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@artemredkin artemredkin

Labels
None yet
Projects
None yet
Milestone
1.0.0
Development

Successfully merging a pull request may close this issue.

Tolerate futures from arbitrary event loops
2 participants
@artemredkin @weissi