fix: bug where successive python changed events trigger creating multiple servers (#557)

Author: karthiknadig

package-lock.json

@@ -10,3 +10,7 @@ export const EXTENSION_ROOT_DIR =
 export const BUNDLED_PYTHON_SCRIPTS_DIR = path.join(EXTENSION_ROOT_DIR, 'bundled');
 export const SERVER_SCRIPT_PATH = path.join(BUNDLED_PYTHON_SCRIPTS_DIR, 'tool', `lsp_server.py`);
 export const DEBUG_SERVER_SCRIPT_PATH = path.join(BUNDLED_PYTHON_SCRIPTS_DIR, 'tool', `_debug_server.py`);
+export const PYTHON_MAJOR = 3;
+export const PYTHON_MINOR = 8;
+export const PYTHON_VERSION = `${PYTHON_MAJOR}.${PYTHON_MINOR}`;
+export const LS_SERVER_RESTART_DELAY = 1000;

src/common/constants.ts

@@ -5,14 +5,16 @@
 import { commands, Disposable, Event, EventEmitter, Uri } from 'vscode';
 import { traceError, traceLog } from './logging';
 import { PythonExtension, ResolvedEnvironment } from '@vscode/python-extension';
+import { PYTHON_MAJOR, PYTHON_MINOR, PYTHON_VERSION } from './constants';
+import { getProjectRoot } from './utilities';
 
 export interface IInterpreterDetails {
     path?: string[];
     resource?: Uri;
 }
 
-const onDidChangePythonInterpreterEvent = new EventEmitter<IInterpreterDetails>();
-export const onDidChangePythonInterpreter: Event<IInterpreterDetails> = onDidChangePythonInterpreterEvent.event;
+const onDidChangePythonInterpreterEvent = new EventEmitter<void>();
+export const onDidChangePythonInterpreter: Event<void> = onDidChangePythonInterpreterEvent.event;
 
 let _api: PythonExtension | undefined;
 async function getPythonExtensionAPI(): Promise<PythonExtension | undefined> {
@@ -23,21 +25,59 @@ async function getPythonExtensionAPI(): Promise<PythonExtension | undefined> {
     return _api;
 }
 
+function sameInterpreter(a: string[], b: string[]): boolean {
+    if (a.length !== b.length) {
+        return false;
+    }
+    for (let i = 0; i < a.length; i++) {
+        if (a[i] !== b[i]) {
+            return false;
+        }
+    }
+    return true;
+}
+
+let serverPython: string[] | undefined;
+function checkAndFireEvent(interpreter: string[] | undefined): void {
+    if (interpreter === undefined) {
+        if (serverPython) {
+            // Python was reset for this uri
+            serverPython = undefined;
+            onDidChangePythonInterpreterEvent.fire();
+            return;
+        } else {
+            return; // No change in interpreter
+        }
+    }
+
+    if (!serverPython || !sameInterpreter(serverPython, interpreter)) {
+        serverPython = interpreter;
+        onDidChangePythonInterpreterEvent.fire();
+    }
+}
+
+async function refreshServerPython(): Promise<void> {
+    const projectRoot = await getProjectRoot();
+    const interpreter = await getInterpreterDetails(projectRoot?.uri);
+    checkAndFireEvent(interpreter.path);
+}
+
 export async function initializePython(disposables: Disposable[]): Promise<void> {
     try {
         const api = await getPythonExtensionAPI();
+
         if (api) {
             disposables.push(
-                api.environments.onDidChangeActiveEnvironmentPath((e) => {
-                    onDidChangePythonInterpreterEvent.fire({ path: [e.path], resource: e.resource?.uri });
+                api.environments.onDidChangeActiveEnvironmentPath(async () => {
+                    await refreshServerPython();
                 }),
             );
 
-            traceLog('Waiting for interpreter from python extension.');
-            onDidChangePythonInterpreterEvent.fire(await getInterpreterDetails());
+            traceLog('Waiting for interpreter from Python extension.');
+            await refreshServerPython();
         }
     } catch (error) {
-        traceError('Error initializing python: ', error);
+        traceError('Error initializing Python: ', error);
     }
 }
 
@@ -69,11 +109,11 @@ export async function runPythonExtensionCommand(command: string, ...rest: any[])
 
 export function checkVersion(resolved: ResolvedEnvironment | undefined): boolean {
     const version = resolved?.version;
-    if (version?.major === 3 && version?.minor >= 8) {
+    if (version?.major === PYTHON_MAJOR && version?.minor >= PYTHON_MINOR) {
         return true;
     }
     traceError(`Python version ${version?.major}.${version?.minor} is not supported.`);
     traceError(`Selected python path: ${resolved?.executable.uri?.fsPath}`);
-    traceError('Supported versions are 3.8 and above.');
+    traceError(`Supported versions are ${PYTHON_VERSION} and above.`);
     return false;
 }

src/common/python.ts

@@ -80,12 +80,12 @@ export async function restartServer(
     serverId: string,
     serverName: string,
     outputChannel: LogOutputChannel,
-    lsClient?: LanguageClient,
+    oldLsClient?: LanguageClient,
 ): Promise<LanguageClient | undefined> {
-    if (lsClient) {
+    if (oldLsClient) {
         traceInfo(`Server: Stop requested`);
         try {
-            await lsClient.stop();
+            await oldLsClient.stop();
         } catch (ex) {
             traceError(`Server: Stop failed: ${ex}`);
         }

src/common/server.ts

@@ -8,6 +8,7 @@ import { traceError, traceInfo, traceLog, traceWarn } from './logging';
 import { EXTENSION_ID } from './constants';
 import { TransportKind } from 'vscode-languageclient/node';
 import { trace } from 'console';
+import { getInterpreterFromSetting } from './utilities';
 
 export interface ISettings {
     cwd: string;
@@ -92,11 +93,6 @@ function getCwd(config: WorkspaceConfiguration, workspace: WorkspaceFolder): str
     return resolveVariables([cwd], 'cwd', workspace)[0];
 }
 
-export function getInterpreterFromSetting(namespace: string, scope?: ConfigurationScope) {
-    const config = getConfiguration(namespace, scope);
-    return config.get<string[]>('interpreter');
-}
-
 export async function getWorkspaceSettings(
     namespace: string,
     workspace: WorkspaceFolder,

src/common/settings.ts

@@ -3,9 +3,9 @@
 
 import * as fs from 'fs-extra';
 import * as path from 'path';
-import { env, LogLevel, Uri, WorkspaceFolder } from 'vscode';
+import { ConfigurationScope, env, LogLevel, Uri, WorkspaceFolder } from 'vscode';
 import { Trace, TraceValues } from 'vscode-jsonrpc/node';
-import { getWorkspaceFolders, isVirtualWorkspace } from './vscodeapi';
+import { getConfiguration, getWorkspaceFolders, isVirtualWorkspace } from './vscodeapi';
 import { DocumentSelector } from 'vscode-languageclient';
 
 function logLevelToTrace(logLevel: LogLevel): Trace {
@@ -77,3 +77,8 @@ export function getDocumentSelector(): DocumentSelector {
               { scheme: 'vscode-notebook-cell', language: 'python' },
           ];
 }
+
+export function getInterpreterFromSetting(namespace: string, scope?: ConfigurationScope) {
+    const config = getConfiguration(namespace, scope);
+    return config.get<string[]>('interpreter');
+}

src/common/utilities.ts

@@ -9,17 +9,17 @@ import { initializePython, onDidChangePythonInterpreter } from './common/python'
 import {
     checkIfConfigurationChanged,
     getExtensionSettings,
-    getInterpreterFromSetting,
     getWorkspaceSettings,
     ISettings,
     logDefaultFormatter,
     logLegacySettings,
 } from './common/settings';
 import { loadServerDefaults } from './common/setup';
-import { getProjectRoot } from './common/utilities';
+import { getInterpreterFromSetting, getProjectRoot } from './common/utilities';
 import { createOutputChannel, onDidChangeConfiguration, registerCommand } from './common/vscodeapi';
 import { registerEmptyFormatter } from './common/nullFormatter';
 import { registerLanguageStatusItem, updateStatus } from './common/status';
+import { LS_SERVER_RESTART_DELAY, PYTHON_VERSION } from './common/constants';
 
 let lsClient: LanguageClient | undefined;
 export async function activate(context: vscode.ExtensionContext): Promise<void> {
@@ -39,19 +39,33 @@ export async function activate(context: vscode.ExtensionContext): Promise<void>
     traceLog(`Module: ${serverInfo.module}`);
     traceVerbose(`Configuration: ${JSON.stringify(serverInfo)}`);
 
+    let isRestarting = false;
+    let restartTimer: NodeJS.Timeout | undefined;
     const runServer = async () => {
-        const projectRoot = await getProjectRoot();
-        const workspaceSetting = await getWorkspaceSettings(serverId, projectRoot, true);
-        if (workspaceSetting.interpreter.length === 0) {
-            updateStatus(vscode.l10n.t('Please select a Python interpreter.'), vscode.LanguageStatusSeverity.Error);
-            traceError(
-                'Python interpreter missing:\r\n' +
-                    '[Option 1] Select python interpreter using the ms-python.python.\r\n' +
-                    `[Option 2] Set an interpreter using "${serverId}.interpreter" setting.\r\n`,
-                'Please use Python 3.8 or greater.',
-            );
-        } else {
-            lsClient = await restartServer(workspaceSetting, serverId, serverName, outputChannel, lsClient);
+        if (isRestarting) {
+            if (restartTimer) {
+                clearTimeout(restartTimer);
+            }
+            restartTimer = setTimeout(runServer, LS_SERVER_RESTART_DELAY);
+            return;
+        }
+        isRestarting = true;
+        try {
+            const projectRoot = await getProjectRoot();
+            const workspaceSetting = await getWorkspaceSettings(serverId, projectRoot, true);
+            if (workspaceSetting.interpreter.length === 0) {
+                updateStatus(vscode.l10n.t('Please select a Python interpreter.'), vscode.LanguageStatusSeverity.Error);
+                traceError(
+                    'Python interpreter missing:\r\n' +
+                        '[Option 1] Select python interpreter using the ms-python.python (select interpreter command).\r\n' +
+                        `[Option 2] Set an interpreter using "${serverId}.interpreter" setting.\r\n`,
+                    `Please use Python ${PYTHON_VERSION} or greater.`,
+                );
+            } else {
+                lsClient = await restartServer(workspaceSetting, serverId, serverName, outputChannel, lsClient);
+            }
+        } finally {
+            isRestarting = false;
         }
     };