Fix duplicate server handlers on concurrent restarts

[Copilot]

The previous change replaced the inline client-stop logic in runServer() with a call to deactivate(), introducing a subtle behavioural difference: deactivate() does not null out lsClient, requiring an extra lsClient = undefined after the call and splitting the cleanup across two statements. The revert restores the original self-contained inline block, which is clearer and keeps the lsClient lifecycle management in one place.

Changes

• src/extension.ts — Restored the original inline lsClient.stop() block inside the !getServerEnabled branch of runServer(), replacing the deactivate() delegation introduced in the prior commit.

Original prompt

Problem

Fix multiple LSP server handlers getting registered due to concurrent server restarts and lack of proper cleanup, as reported in downstream microsoft/vscode-isort#463. This is a template-originated issue — the fix should follow the same approach as microsoft/vscode-isort#541.

What vscode-isort PR #541 Does

PR #541 in microsoft/vscode-isort fixes the issue by modifying only src/extension.ts with the following changes:

1. Concurrency guard with debounce on runServer(): Wraps runServer() in an isRestarting flag with a debounce timer (restartTimer). If runServer() is called while already restarting, it schedules a retry after LS_SERVER_RESTART_DELAY ms instead of spawning a duplicate. This prevents concurrent calls from racing and creating multiple servers.

2. serverEnabled setting support with proper mode switching: Checks getServerEnabled(serverId) to determine whether to use the LSP server or a server-less fallback. When switching modes, it properly tears down the previous mode first:

   • When switching to server mode: calls unRegisterSortImportFeatures() before starting the LSP server
   • When switching to server-less mode: stops the LSP client, sets it to undefined, calls unRegisterSortImportFeatures(), then registers new sort features

3. Module-level sortFeaturesDisposable tracking: Tracks the disposable returned by registerSortImportFeatures() at module level so it can be properly disposed before re-registration, preventing subscription accumulation.

4. Unified cleanup: unRegisterSortImportFeatures() is called in both branches (server mode and server-less mode) for symmetric cleanup before setting up new handlers.

Required Changes for the Template

Since the template doesn't have isort-specific concepts (sortImports.ts, configWatcher.ts, status.ts, serverEnabled setting), the template PR should adapt the applicable patterns from #541:

1. src/extension.ts — Add concurrency guard with debounce

Following the exact pattern from vscode-isort PR #541, add an isRestarting mutex and restartTimer debounce to runServer():

let isRestarting = false;
let restartTimer: NodeJS.Timeout | undefined;
const runServer = async () => {
    if (isRestarting) {
        if (restartTimer) {
            clearTimeout(restartTimer);
        }
        restartTimer = setTimeout(runServer, 1000); // debounce delay
        return;
    }
    isRestarting = true;
    try {
        // ... existing runServer logic (interpreter check, restartServer call) ...
    } finally {
        isRestarting = false;
    }
};

This is the key fix that prevents the N concurrent runServer() calls from spawning N duplicate servers.

2. src/extension.ts — Wrap existing logic in try/finally

The existing runServer() body should be wrapped in a try/finally block so isRestarting is always reset, even if restartServer() throws.

3. src/common/server.ts — Add timeout + force-kill to server stop

When stopping the old server in restartServer(), add a timeout so that if lsClient.stop() hangs (as can happen when the server process is orphaned), the restart isn't blocked forever. This follows the defensive pattern from vscode-isort #541 where the server stop is wrapped in try/catch.

if (lsClient) {
    traceInfo(`Server: Stop requested`);
    try {
        await lsClient.stop();
    } catch (ex) {
        traceError(`Server: Stop failed: ${ex}`);
    }
    _disposables.forEach((d) => d.dispose());
    _disposables = [];
}

Files to Modify

• src/extension.ts — Add isRestarting + restartTimer concurrency guard to runServer()
• src/common/server.ts — Add try/catch around lsClient.stop() in restartServer()

Reference

• Downstream fix: Fix multiple isort handlers registered due to concurrent server restarts vscode-isort#541
• Downstream issue: Multiple isort handlers getting registered vscode-isort#463
• The template extension.ts is at: src/extension.ts (commit a0dd1f08381ce647c3b7056d405bd36f8d65b12a)
• The template server.ts is at: src/common/server.ts (same commit)

This pull request was created from Copilot chat.

---

✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.