-
Notifications
You must be signed in to change notification settings - Fork 15
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
RPC: Ark: Switch to blocking requests to avoid concurrent R evaluations #689
Comments
Also the propagation and handling of |
Currently ReadConsole pushes prompt events to signal the end of a top-level command. The variable comm listens to these events to refresh the bindings in an Or alternatively run a handler on the R tread after each ReadConsole tick to grab the state we're interested in. Could be set up as a new kind of idle task that is guaranteed to run after an iteration. |
I think we need to be a bit more careful about the timing of evaluation of backend RPCs on the R side. This concerns the
callMethod()
mechanism (which makes a request from typescript via the UI comm) and more generally any requests performed by a comm. These shouldn't run concurrently with the R interpreter because we are running complex functions that have not been designed for reentrancy or preemption (more precisely interrupt-time, which happens at somewhat controlled points, but in practice we should consider it happens at any time).RPCs are becoming our main way of calling R code from positron-r because they support serialisation of arguments whereas
execute()
only support textual arguments. Because of this we are bound to be calling complex code, for example code that loads an R package. This is potentially bad for many reasons, for instance ifloadNamespace()
is in the middle of loading another package at that moment.Same concern for comm RPCs, for instance for the connection comm. The
.ps.connection_
API is accessible from R and might be called by a package or a user. Then a message comes in and runs connection code preemptively (at interrupt time). This means the connection R code should be reentrant but we can't reasonably expect this. I think we'll eventually end up with weird state corruption. It doesn't even require mutable state to get corruption, it just suffices for state to be split in multiple places and change preemptively between two lookups.Thankfully I don't think we'll lose much by making these requests synchronous. They are currently made via
comm_msg
which is queued on the Shell socket. The problem comes from closing thecomm_msg
request too fast, which allows Shell to invoke the next request, possibly anexecute_request
or another comm RPC. To avoid that we just need to block during the processing of the comm message to preventShell
from executing the next request while the comm is calling R code.The text was updated successfully, but these errors were encountered: