[vscode-lldb] Restart server when lldb-dap binary has changed #159797

royitaqi · 2025-09-19T15:42:51Z

Motiviation

This helps the development of the lldb-dap binary. For example, when testing a locally built lldb-dap binary, one probably wants to restart the server after each build in order to use the latest binary. Not doing so leads to confusion like "why the new lldb-dap isn't doing what I just changed?".

This patch adds dependency to the chokidar package. The reason is that we need something to detect changes to the lldb-dap binary file and chokidar appears to be the most reliable package to do so. An alternative solution which doesn't require adding dependencies is discussed below (solution 1).

Two different solutions considered

Solution 1: Restart server when lldb-dap binary's modification time changes. #159481 implements solution 1.

Solution 2: Restart server when lldb-dap binary has changed (as detected by a file system watcher). This patch implements solution 2 (using chokidar).

This patch (solution 2)

If the lldb-dap binary has changed, the next time the user start a debug session, a dialog box will show up and prompt the user to restart the server. Depend on what has changed, the dialog box will show different content (see below)

When both the lldb-dap binary and the arguments have changed:

When only the lldb-dap binary has changed:

When only the arguments have changed (existing):

llvmbot · 2025-09-19T15:43:38Z

@llvm/pr-subscribers-lldb

Author: Roy Shi (royitaqi)

Changes

Motiviation

This helps the development of the lldb-dap binary. For example, when testing a locally built lldb-dap binary, one probably wants to restart the server after each build in order to use the latest binary. Not doing so leads to confusion like "why the new lldb-dap isn't doing what I just changed?".

Two different solutions considered

Solution 1: Restart server when lldb-dap binary's modification time changes. #159481 implements solution 1.

Solution 2: Restart server when lldb-dap binary has changed (as detected by vscode.workspace.createFileSystemWatcher). This patch implements solution 2.

This patch (solution 2)

If the lldb-dap binary has changed, the next time the user start a debug session, a dialog box will show up and prompt the user to restart the server.

Note that the message and detail of the dialog box is different from the existing one (which is shown when lldb-dap path/args/env has changed), shown below.

Full diff: https://github.com/llvm/llvm-project/pull/159797.diff

1 Files Affected:

(modified) lldb/tools/lldb-dap/src-ts/lldb-dap-server.ts (+64-13)

diff --git a/lldb/tools/lldb-dap/src-ts/lldb-dap-server.ts b/lldb/tools/lldb-dap/src-ts/lldb-dap-server.ts
index 774be50053a17..7a1754f4bce6a 100644
--- a/lldb/tools/lldb-dap/src-ts/lldb-dap-server.ts
+++ b/lldb/tools/lldb-dap/src-ts/lldb-dap-server.ts
@@ -1,4 +1,5 @@
 import * as child_process from "node:child_process";
+import * as path from "path";
 import { isDeepStrictEqual } from "util";
 import * as vscode from "vscode";
 
@@ -12,6 +13,10 @@ export class LLDBDapServer implements vscode.Disposable {
   private serverProcess?: child_process.ChildProcessWithoutNullStreams;
   private serverInfo?: Promise<{ host: string; port: number }>;
   private serverSpawnInfo?: string[];
+  // Detects changes to the lldb-dap executable file since the server's startup.
+  private serverFileWatcher?: vscode.FileSystemWatcher;
+  // Indicates whether the lldb-dap executable file has changed since the server's startup.
+  private serverFileChanged?: boolean;
 
   constructor() {
     vscode.commands.registerCommand(
@@ -83,6 +88,18 @@ export class LLDBDapServer implements vscode.Disposable {
       });
       this.serverProcess = process;
       this.serverSpawnInfo = this.getSpawnInfo(dapPath, dapArgs, options?.env);
+      this.serverFileChanged = false;
+      // Cannot do `createFileSystemWatcher(dapPath)` for a single file. Have to use `RelativePattern`.
+      // See https://github.com/microsoft/vscode/issues/141011#issuecomment-1016772527
+      this.serverFileWatcher = vscode.workspace.createFileSystemWatcher(
+        new vscode.RelativePattern(
+          vscode.Uri.file(path.dirname(dapPath)),
+          path.basename(dapPath),
+        ),
+      );
+      this.serverFileWatcher.onDidChange(() => {
+        this.serverFileChanged = true;
+      });
     });
     return this.serverInfo;
   }
@@ -100,20 +117,34 @@ export class LLDBDapServer implements vscode.Disposable {
     args: string[],
     env: NodeJS.ProcessEnv | { [key: string]: string } | undefined,
   ): Promise<boolean> {
-    if (!this.serverProcess || !this.serverInfo || !this.serverSpawnInfo) {
+    if (
+      !this.serverProcess ||
+      !this.serverInfo ||
+      !this.serverSpawnInfo ||
+      !this.serverFileWatcher ||
+      this.serverFileChanged === undefined
+    ) {
       return true;
     }
 
-    const newSpawnInfo = this.getSpawnInfo(dapPath, args, env);
-    if (isDeepStrictEqual(this.serverSpawnInfo, newSpawnInfo)) {
-      return true;
-    }
+    // Check if the server has changed. If so, generate message and detail for user prompt.
+    const messageAndDetail = (() => {
+      if (this.serverFileChanged) {
+        return {
+          message:
+            "The lldb-dap binary has changed. Would you like to restart the server?",
+          detail: `An existing lldb-dap server (${this.serverProcess.pid}) is running with an old binary.
 
-    const userInput = await vscode.window.showInformationMessage(
-      "The arguments to lldb-dap have changed. Would you like to restart the server?",
-      {
-        modal: true,
-        detail: `An existing lldb-dap server (${this.serverProcess.pid}) is running with different arguments.
+Restarting the server will interrupt any existing debug sessions and start a new server.`,
+        };
+      }
+
+      const newSpawnInfo = this.getSpawnInfo(dapPath, args, env);
+      if (!isDeepStrictEqual(this.serverSpawnInfo, newSpawnInfo)) {
+        return {
+          message:
+            "The arguments to lldb-dap have changed. Would you like to restart the server?",
+          detail: `An existing lldb-dap server (${this.serverProcess.pid}) is running with different arguments.
 
 The previous lldb-dap server was started with:
 
@@ -124,15 +155,31 @@ The new lldb-dap server will be started with:
 ${newSpawnInfo.join(" ")}
 
 Restarting the server will interrupt any existing debug sessions and start a new server.`,
+        };
+      }
+
+      return null;
+    })();
+
+    // If the server hasn't changed, continue startup without killing it.
+    if (messageAndDetail === null) {
+      return true;
+    }
+
+    // The server has changed. Prompt the user to restart it.
+    const { message, detail } = messageAndDetail;
+    const userInput = await vscode.window.showInformationMessage(
+      message,
+      {
+        modal: true,
+        detail,
       },
       "Restart",
       "Use Existing",
     );
     switch (userInput) {
       case "Restart":
-        this.serverProcess.kill();
-        this.serverProcess = undefined;
-        this.serverInfo = undefined;
+        this.dispose();
         return true;
       case "Use Existing":
         return true;
@@ -156,6 +203,10 @@ Restarting the server will interrupt any existing debug sessions and start a new
     if (this.serverProcess === process) {
       this.serverProcess = undefined;
       this.serverInfo = undefined;
+      this.serverSpawnInfo = undefined;
+      this.serverFileWatcher?.dispose();
+      this.serverFileWatcher = undefined;
+      this.serverFileChanged = undefined;
     }
   }

walter-erquinigo · 2025-09-19T16:07:09Z

lldb/tools/lldb-dap/src-ts/lldb-dap-server.ts

      });
      this.serverProcess = process;
      this.serverSpawnInfo = this.getSpawnInfo(dapPath, dapArgs, options?.env);
+      this.serverFileChanged = false;


try using https://www.npmjs.com/package/chokidar

~~Just to clarify: vscode.workspace.createFileSystemWatcher now works on my machine. Are you suggesting to use chokidar instead?~~

~~Also, even with chokidar, I will have to have this.serverFileChanged, so that we know that the lldb-dap binary has changed at the start of the next debug session. Make sense?~~

Updated the patch to use chokidar.

royitaqi · 2025-09-19T22:03:18Z

@walter-erquinigo: Updated the patch to use chokidar. It worked out of the box, which is nice~

Any other suggestions from you?

…og box

…er-when-modified-2

royitaqi · 2025-09-22T17:28:57Z

@walter-erquinigo: Gentle ping for a review.

Since your last review, I have:

Used chokidar.
Rebased on latest main.
When both the lldb-dap binary and the arguments have changed, I made the dialog box show both changes (see the first screenshot in the patch's description).

JDevlieghere · 2025-09-22T17:40:17Z

When you merge this, please include a little bit of motivation for introducing a new external dependency. I skimmed the package and it seems well-established with limited transitive dependencies, but generally speaking we should have a pretty high bar for adding new dependencies.

royitaqi · 2025-09-22T17:57:16Z

When you merge this, please include a little bit of motivation for introducing a new external dependency.

I have just updated the "Motivation" section of the patch description to include an explanation. Feel free to LMK if you want me to change anything there.

I skimmed the package and it seems well-established with limited transitive dependencies, but generally speaking we should have a pretty high bar for adding new dependencies.

If we want no/less dependency, we can use #159481 instead. I can update it so that it only uses the fs package (which IIUC should be included by default). However, that solution uses a different approach (checks the modification time of the lldb-dap binary) and is considered less reliable per @walter-erquinigo's comment (I'm still yet to learn why that is the case).

walter-erquinigo

cool! thanks!

About the reliability, all I can tell you is that I used to manage a few vscode extensions and tracking modification times was not always working on all systems. At least that's why my users reported, but I never got access to those systems so I couldn't really know the underlying issues. But when I switched to using well established libraries for this, my problems went away...

royitaqi · 2025-09-23T14:09:30Z

@walter-erquinigo: Thanks for sharing the context about the reliability. I appreciate it. Will keep that in mind.

z2oh · 2025-09-24T18:01:39Z

I started hitting an error about missing chokidar module today and came across this PR when searching. My error is:

2025-09-24 10:57:12.237 [info] ExtensionService#_doActivateExtension llvm-vs-code-extensions.lldb-dap, startup: false, activationEvent: 'onLanguage:swift', root cause: swiftlang.swift-vscode
2025-09-24 10:57:12.240 [error] Activating extension llvm-vs-code-extensions.lldb-dap failed due to an error:
2025-09-24 10:57:12.240 [error] Error: Cannot find module 'chokidar'
Require stack:
- /Users/jeremy/.vscode/extensions/llvm-vs-code-extensions.lldb-dap-0.2.17/out/lldb-dap-server.js
- /Users/jeremy/.vscode/extensions/llvm-vs-code-extensions.lldb-dap-0.2.17/out/extension.js
- /Applications/Visual Studio Code.app/Contents/Resources/app/out/vs/workbench/api/node/extensionHostProcess.js

I'm not too familiar with the node ecosystem, but is there some missing step here that should be distributing or installing chokidar? Reinstalling the extension didn't help, but manually running npm install in /Users/jeremy/.vscode/extensions/llvm-vs-code-extensions.lldb-dap-0.2.17 pulls down this dependency and the extension can start.

(cc @JDevlieghere ; unless I'm just holding it wrong I think ldb-dap-0.2.17 is broken)

JDevlieghere · 2025-09-24T20:42:24Z

Thanks, fixed by #160598

royitaqi · 2025-09-24T22:09:02Z

Sorry for the trouble, guys~

@matthewbastien, @JDevlieghere: Thanks for the quick fix. I appreciate it~!

…59797) # Motiviation This helps the development of the lldb-dap binary. For example, when testing a locally built lldb-dap binary, one probably wants to restart the server after each build in order to use the latest binary. Not doing so leads to confusion like "why the new lldb-dap isn't doing what I just changed?". This patch adds dependency to the `chokidar` package. The reason is that we need something to detect changes to the `lldb-dap` binary file and `chokidar` appears to be the most reliable package to do so. An alternative solution which doesn't require adding dependencies is discussed below (solution 1). # Two different solutions considered Solution 1: Restart server when lldb-dap binary's modification time changes. llvm#159481 implements solution 1. Solution 2: Restart server when lldb-dap binary has changed (as detected by a file system watcher). This patch implements solution 2 (using `chokidar`). # This patch (solution 2) If the lldb-dap binary has changed, the next time the user start a debug session, a dialog box will show up and prompt the user to restart the server. Depend on what has changed, the dialog box will show different content (see below) * When both the lldb-dap binary and the arguments have changed: <img width="520" height="357" alt="diff_args_and_binary" src="https://github.com/user-attachments/assets/6580e05f-05c3-4d80-8c1a-9c3abf279218" /> * When only the lldb-dap binary has changed: <img width="260" height="384" alt="diff_binary" src="https://github.com/user-attachments/assets/933b8987-9c21-44f3-ad68-57b8eeb58525" /> * When only the arguments have changed (existing): <img width="520" height="343" alt="diff_args" src="https://github.com/user-attachments/assets/c0e6771c-ad32-4fb8-b5df-b34cce48ed1a" /> (cherry picked from commit 6007a4d)

[vscode-lldb] Restart server when the lldb-dap binary has changed

e4d4d1c

royitaqi requested a review from JDevlieghere as a code owner September 19, 2025 15:42

llvmbot added lldb lldb-dap labels Sep 19, 2025

walter-erquinigo reviewed Sep 19, 2025

View reviewed changes

royitaqi mentioned this pull request Sep 19, 2025

[vscode-lldb] Restart server when the lldb-dap binary's modification time has changed #159481

Closed

royitaqi requested review from aperez, clayborg, dmpots, jeffreytan81 and kusmour September 19, 2025 20:44

Use chokidar instead

eeae075

royitaqi added 2 commits September 19, 2025 15:28

Show both the binary change and the arguments change in the same dial…

2b6e288

…og box

Merge remote-tracking branch 'origin/main' into lldb-dap-restart-serv…

35ace92

…er-when-modified-2

royitaqi requested a review from walter-erquinigo September 22, 2025 22:20

walter-erquinigo approved these changes Sep 23, 2025

View reviewed changes

royitaqi and others added 3 commits September 23, 2025 07:14

Remove out-dated inline comments

de3c385

Merge branch 'main' into lldb-dap-restart-server-when-modified-2

6215c1c

Merge branch 'main' into lldb-dap-restart-server-when-modified-2

e31b9b6

royitaqi merged commit 6007a4d into llvm:main Sep 23, 2025
9 checks passed

[vscode-lldb] Restart server when lldb-dap binary has changed #159797

[vscode-lldb] Restart server when lldb-dap binary has changed #159797

Uh oh!

Conversation

royitaqi commented Sep 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Motiviation

Two different solutions considered

This patch (solution 2)

Uh oh!

llvmbot commented Sep 19, 2025

Motiviation

Two different solutions considered

This patch (solution 2)

Uh oh!

walter-erquinigo Sep 19, 2025

Choose a reason for hiding this comment

Uh oh!

royitaqi Sep 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

royitaqi commented Sep 19, 2025

Uh oh!

royitaqi commented Sep 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

JDevlieghere commented Sep 22, 2025

Uh oh!

royitaqi commented Sep 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

walter-erquinigo left a comment

Choose a reason for hiding this comment

Uh oh!

royitaqi commented Sep 23, 2025

Uh oh!

Uh oh!

z2oh commented Sep 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

JDevlieghere commented Sep 24, 2025

Uh oh!

royitaqi commented Sep 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

royitaqi commented Sep 19, 2025 •

edited

Loading

royitaqi Sep 19, 2025 •

edited

Loading

royitaqi commented Sep 22, 2025 •

edited

Loading

royitaqi commented Sep 22, 2025 •

edited

Loading

z2oh commented Sep 24, 2025 •

edited

Loading

royitaqi commented Sep 24, 2025 •

edited

Loading