Exit R gracefully exits the Reticulate Python session #5100
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.
Addresses #5083
Requires Ark side: posit-dev/ark#603
This is built on top of #5056 so should merge it before this one.
In the Ark side we added a finalizer using
reg.finalizer
to thereticulate
namespace. So whenever the reticulate namespace is unloaded (or when the R session is about to exit), we'll send a shutdown event to the reticulate extension. The R side waits for the event to be processed and for the comm client to be disposed before really finalizing the session. Thus making sure that the reticulate Python session has gracefully exited before ending the R session - which would cause IPykernel to crash otherwise.I'm not sure this is the best approach although it's the one that doesn't need to introduce any new concepts.
We could also:
Add API's to register shutdown callbacks to
positron.RuntimeSession
, these callbacks would beawait
before calling the sessionshutdown
routines.Perhaps exposing the
Exiting
RuntimeState. Although this might not be enough, because we really need to wait for the Python session to end before proceeding.positron/src/vs/workbench/services/languageRuntime/common/languageRuntimeService.ts
Lines 299 to 301 in bf3de59
Introducing the concept of parent/child sessions and 'parents' would be resposnible for shutting down their child sessions before shutting them down.
@jmcphers do you have a sense if the current approach is reasonable?