Skip to content
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.

Sign up for GitHub

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

Only await LSP stop when kernel is online #1144

Merged
merged 1 commit into from
Aug 24, 2023
Merged

Only await LSP stop when kernel is online #1144

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 21 additions & 13 deletions extensions/positron-r/src/lsp.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
*--------------------------------------------------------------------------------------------*/

import * as vscode from 'vscode';
import * as positron from 'positron';
import { PromiseHandles } from './util';

import {
Expand Down Expand Up @@ -129,9 +130,12 @@ export class ArkLsp implements vscode.Disposable {
/**
* Stops the client instance.
*
* @param awaitStop If true, waits for the client to stop before returning.
* This should be set to `true` if the server process is still running, and
* `false` if the server process has already exited.
* @returns A promise that resolves when the client has been stopped.
*/
public async deactivate() {
public async deactivate(awaitStop: boolean) {
if (!this._client) {
// No client to stop, so just resolve
return;
Expand All @@ -147,18 +151,22 @@ export class ArkLsp implements vscode.Disposable {
// partially initialized client.
await this._initializing;

// The promise returned by `stop()` never resolves if the server side is
// disconnected, so rather than awaiting it, we wait for the client to
// change state to `stopped`, which does happen reliably.
const promise = new Promise<void>((resolve) => {
const disposable = this._client!.onDidChangeState((event) => {
if (event.newState === State.Stopped) {
resolve();
disposable.dispose();
}
const promise = awaitStop ?
// If the kernel hasn't exited, we can just await the promise directly
this._client!.stop() :
// The promise returned by `stop()` never resolves if the server
// side is disconnected, so rather than awaiting it when the runtime
// has exited, we wait for the client to change state to `stopped`,
// which does happen reliably.
new Promise<void>((resolve) => {
const disposable = this._client!.onDidChangeState((event) => {
if (event.newState === State.Stopped) {
resolve();
disposable.dispose();
}
});
this._client!.stop();
});
this._client!.stop();
});

// Don't wait more than a couple of seconds for the client to stop.
const timeout = new Promise<void>((_, reject) => {
Expand All @@ -183,6 +191,6 @@ export class ArkLsp implements vscode.Disposable {
*/
async dispose() {
this.activationDisposables.forEach(d => d.dispose());
await this.deactivate();
await this.deactivate(false);
}
}
6 changes: 3 additions & 3 deletions extensions/positron-r/src/runtime.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ export class RRuntime implements positron.LanguageRuntime, vscode.Disposable {
async restart(): Promise<void> {
if (this._kernel) {
// Stop the LSP client before restarting the kernel
await this._lsp.deactivate();
await this._lsp.deactivate(true);
return this._kernel.restart();
} else {
throw new Error('Cannot restart; kernel not started');
Expand All @@ -139,7 +139,7 @@ export class RRuntime implements positron.LanguageRuntime, vscode.Disposable {
async shutdown(): Promise<void> {
if (this._kernel) {
// Stop the LSP client before shutting down the kernel
await this._lsp.deactivate();
await this._lsp.deactivate(true);
return this._kernel.shutdown();
} else {
throw new Error('Cannot shutdown; kernel not started');
Expand Down Expand Up @@ -195,7 +195,7 @@ export class RRuntime implements positron.LanguageRuntime, vscode.Disposable {
if (this._kernel) {
this._kernel.emitJupyterLog(`Stopping Positron LSP server`);
}
await this._lsp.deactivate();
await this._lsp.deactivate(false);
});
}
}
Expand Down