Run the Help comm on the R thread#1284
Open
lionel- wants to merge 1 commit into
Open
Conversation
This file contains hidden or 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
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.
Branched from #1283
Progress towards #689 and #691
See #1075 for context: we're moving comms out of their own threads to run on the R thread, to simplify the concurrency structure of Ark and remove the risk of concurrent R evaluations which are UB (although these are now also tackled under a different angle, see #1222).
The help comm no longer has its own thread and event loop. Instead it implements
CommHandlerand its handlers get run on the R thread.Like the UI comm, it's a pinned comm that is accessible via methods on Console, so it uses the patterns established in Simplify ownership of the list of comms #1283.
New kernel-level integration test for the Help comm (progress towards Migrate existing comm tests to protocol-level tests #1074).