-
Notifications
You must be signed in to change notification settings - Fork 134
Description
I can't reproduce this race condition all the time but when it does happen the behaviour is sticky for a while. What happens is that after leaving a debugging session, the debugger prompt is incorrectly displayed:
Browse[1]>
Evaluating something (even empty evaluation) then restores the expected prompt:
>
The race condition
Here is the flow of the prompt update when we leave a debugging session:
-
Ark emits the configuration for the next prompt via the UI comm: https://github.com/posit-dev/ark/blob/af8b0849c0ab294fbf728ef1ef4b55c307f39346/crates/ark/src/interface.rs#L1261-L1266
This happens right before closing the active execute_request.
-
On the frontend side, UI client handling propagates:
positron/src/vs/workbench/services/runtimeSession/common/activeRuntimeSession.ts
Line 202 in 6c45e8c
this._register(uiClient.onDidPromptState(event => { -
Session handling propagates:
positron/src/vs/workbench/services/runtimeSession/common/activeRuntimeSession.ts
Line 202 in 6c45e8c
this._register(uiClient.onDidPromptState(event => { -
Runtime session service handling propagates:
this._register(activeSession.onDidReceiveRuntimeEvent(evt => { -
Runtime service handling emits a PromptConfig event: https://github.com/posit-dev/positron/blob/main/src/vs/workbench/api/browser/positron/mainThreadLanguageRuntime.ts#L207
-
Console Input consumes the event and updates the prompt:
positron/src/vs/workbench/contrib/positronConsole/browser/components/consoleInput.tsx
Line 1066 in 6c45e8c
codeEditorWidget.updateOptions(createLineNumbersOptions());
AFAICT the event propagation on the main thread side is all synchronous. So I think the race condition comes from the very first step, sending an event via the UI comm before closing the active request. The comm event is sent via IOPub whereas the closing of the execute_request is via Shell. If something in the IOPub messaging is a little slower, the prompt update might arrive in the extension host after the closing of the execute request.
To fix this, we could include the prompt information in the response to the execute_request instead of sending it separately.
Weirdness in dynState ownership and synchronisation
Relatedly I've noticed a weirdness in the ownership and synchronisation model of dynState. The sessions on the extension hosts are required to have a dynState containing things like the session name and the prompts, but prompts are never actually updated after initialisation. I think a simpler setup for the dynState would be for it to live entirely on the main thread side, perhaps allowing extensions to retrieve it (although this might not even be needed).