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.

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

fix: only spawn one runner process and shut it down properly #1009

Merged
merged 8 commits into from
Apr 8, 2024
7 changes: 7 additions & 0 deletions packages/safe-ds-lang/src/language/index.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import { RPC_RUNNER_STARTED } from './runner/safe-ds-runner.js';

// Services
export type { SafeDsServices } from './safe-ds-module.js';
export { createSafeDsServices } from './safe-ds-module.js';
Expand All @@ -17,3 +19,8 @@ export { locationToString, positionToString, rangeToString } from '../helpers/lo

// Messages
export * as messages from './runner/messages.js';

// Remote procedure calls
export const rpc = {
runnerStarted: RPC_RUNNER_STARTED,
};
31 changes: 26 additions & 5 deletions packages/safe-ds-lang/src/language/runner/safe-ds-runner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ import { SafeDsMessagingProvider } from '../lsp/safe-ds-messaging-provider.js';

// Most of the functionality cannot be tested automatically as a functioning runner setup would always be required

export const RPC_RUNNER_STARTED = 'runner/started';

const LOWEST_SUPPORTED_VERSION = '0.8.0';
const LOWEST_UNSUPPORTED_VERSION = '0.9.0';
const npmVersionRange = `>=${LOWEST_SUPPORTED_VERSION} <${LOWEST_UNSUPPORTED_VERSION}`;
Expand Down Expand Up @@ -62,8 +64,7 @@ export class SafeDsRunner {
this.registerMessageLoggingCallbacks();

services.workspace.SettingsProvider.onRunnerCommandUpdate(async (newValue) => {
this.updateRunnerCommand(newValue);
await this.startPythonServer();
await this.updateRunnerCommand(newValue);
});

services.shared.lsp.Connection?.onShutdown(async () => {
Expand Down Expand Up @@ -117,17 +118,32 @@ export class SafeDsRunner {
);
}, 'runtime_error');
}
/* c8 ignore stop */

async connectToPort(port: number): Promise<void> {
if (this.isPythonServerAvailable()) {
return;
}

this.port = port;
try {
await this.connectToWebSocket();
} catch (error) {
await this.stopPythonServer();
}
}

/**
* Change the command to start the runner process. This will not cause the runner process to restart, if it is already running.
*
* @param command New Runner Command.
*/
/* c8 ignore start */
public updateRunnerCommand(command: string | undefined): void {
public async updateRunnerCommand(command: string | undefined): Promise<void> {
if (command) {
this.runnerCommand = command;
await this.startPythonServer();
if (this.isPythonServerAvailable()) {
await this.messaging.sendNotification(RPC_RUNNER_STARTED, this.port);
}
}
}

Expand All @@ -136,6 +152,11 @@ export class SafeDsRunner {
* Uses the 'safe-ds.runner.command' setting to execute the process.
*/
public async startPythonServer(): Promise<void> {
if (this.isPythonServerAvailable()) {
this.info('As the Safe-DS Runner is currently successfully running, no attempt to start it will be made');
return;
}

this.acceptsConnections = false;
const runnerCommandParts = this.runnerCommand.split(/\s/u);
const runnerCommand = runnerCommandParts.shift()!; // After shift, only the actual args are left
Expand Down
51 changes: 12 additions & 39 deletions packages/safe-ds-vscode/src/extension/mainClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import * as path from 'node:path';
import * as vscode from 'vscode';
import type { LanguageClientOptions, ServerOptions } from 'vscode-languageclient/node.js';
import { LanguageClient, TransportKind } from 'vscode-languageclient/node.js';
import { ast, createSafeDsServices, getModuleMembers, messages, SafeDsServices } from '@safe-ds/lang';
import { ast, createSafeDsServices, getModuleMembers, messages, rpc, SafeDsServices } from '@safe-ds/lang';
import { NodeFileSystem } from 'langium/node';
import { initializeLog, logError, logOutput, printOutputMessage } from './output.js';
import crypto from 'crypto';
Expand All @@ -23,22 +23,25 @@ let lastSuccessfulPipelineNode: ast.SdsPipeline | undefined;
// This function is called when the extension is activated.
export const activate = async function (context: vscode.ExtensionContext) {
initializeLog();
client = startLanguageClient(context);
const runnerCommandSetting = vscode.workspace.getConfiguration('safe-ds.runner').get<string>('command')!; // Default is set
services = (
await createSafeDsServices(NodeFileSystem, {
logger: {
info: logOutput,
error: logError,
},
runnerCommand: runnerCommandSetting,
userMessageProvider: {
showErrorMessage: vscode.window.showErrorMessage,
},
})
).SafeDs;
await services.runtime.Runner.startPythonServer();
acceptRunRequests(context);

client = createLanguageClient(context);
client.onNotification(rpc.runnerStarted, async (port: number) => {
await services.runtime.Runner.connectToPort(port);
});
await client.start();

registerVSCodeCommands(context);
};

// This function is called when the extension is deactivated.
Expand All @@ -50,7 +53,7 @@ export const deactivate = async function (): Promise<void> {
return;
};

const startLanguageClient = function (context: vscode.ExtensionContext): LanguageClient {
const createLanguageClient = function (context: vscode.ExtensionContext): LanguageClient {
const serverModule = context.asAbsolutePath(path.join('dist', 'extension', 'mainServer.cjs'));
// The debug options for the server
// --inspect=6009: runs the server in Node's Inspector mode so VS Code can attach to the server for debugging.
Expand Down Expand Up @@ -82,19 +85,8 @@ const startLanguageClient = function (context: vscode.ExtensionContext): Languag
outputChannel: vscode.window.createOutputChannel('Safe-DS Language Client', 'log'),
};

// Create the language client and start the client.
const result = new LanguageClient('safe-ds', 'Safe-DS', serverOptions, clientOptions);

// Start the client. This will also launch the server
result.start();
return result;
};

const acceptRunRequests = async function (context: vscode.ExtensionContext) {
// Register VS Code Entry Points
registerVSCodeCommands(context);
// Register watchers
registerVSCodeWatchers();
// Create the language client
return new LanguageClient('safe-ds', 'Safe-DS', serverOptions, clientOptions);
};

const registerVSCodeCommands = function (context: vscode.ExtensionContext) {
Expand Down Expand Up @@ -420,25 +412,6 @@ const validateDocuments = async function (
return undefined;
};

const registerVSCodeWatchers = function () {
vscode.workspace.onDidChangeConfiguration((event) => {
if (event.affectsConfiguration('safe-ds.runner.command')) {
// Try starting runner
logOutput('Safe-DS Runner Command was updated');
services.runtime.Runner.updateRunnerCommand(
vscode.workspace.getConfiguration('safe-ds.runner').get<string>('command')!,
);
if (!services.runtime.Runner.isPythonServerAvailable()) {
services.runtime.Runner.startPythonServer();
} else {
logOutput(
'As the Safe-DS Runner is currently successfully running, no attempt to start it will be made',
);
}
}
});
};

const isRangeEqual = function (lhs: Range, rhs: Range): boolean {
return (
lhs.start.character === rhs.start.character &&
Expand Down