Use trio memory channels! #56
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
added 8 commits
February 14, 2019 13:08
For now stop `.aclose()`-ing all async gens on portal close since it can cause hangs and other weird behaviour if another task operates on the same instance. See https://bugs.python.org/issue32526.
In combination with `.aclose()`-ing the async gen instance returned from `Portal.run()` this demonstrates the python bug: https://bugs.python.org/issue32526 I've commented out the line that triggers the bug for now since this case provides motivation for adding our own `trio.abc.ReceiveMemoryChannel` implementation to be used instead of async gens directly (returned from `Portal.run()`) since the latter is **not** task safe.
As mentioned in prior commits there's currently a bug in Python that make async gens **not** task safe. Since this is the core cause of almost all recent problems, instead implement our own async iterator derivative of `trio.abc.ReceiveChannel` by wrapping a `trio._channel.MemoryReceiveChannel`. This fits more natively with the memory channel API in ``trio`` and adds potentially more flexibility for possible bidirectional inter-actor streaming in the future. Huge thanks to @oremanj and of course @njsmith for guidance on this one!
goodboy
commented
Feb 16, 2019
goodboy
commented
Feb 16, 2019
goodboy
commented
Feb 16, 2019
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
triodeprecated the oldQueuein0.11.0and introduced the newtrio.open_memory_channel()system. Adopt this throughout the core and adjust all tests.In other fun news, we were also suffering from a super subtle bug in Python itself, namely:
https://bugs.python.org/issue32526
This was the root cause of a slew of issues in some UI work in
piker.It turns out (at least with
trio) asyn gen functions are not task safe and can cause all sorts of strange issues if operated on by multiple tasks. Thanks to in depth diagnosis on the gitter channel, the alternative is to introduce an async iterator derivative oftrio.abc.ReceiveChannelwhich wraps an underlying receive memory channel: the deliverer of iter-actor messages to actor-local tasks.Todo:
trioto> 0.10.0?Ping @oremanj @njsmith @vodik!
Please feel free to tear this right apart.
@oremanj @njsmith the main part I would very much appreciate your eyes on is the new async iterator wrapper.