Skip to content

Commit 6007a4d

Browse files
authored
[vscode-lldb] Restart server when lldb-dap binary has changed (#159797)
# 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: <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" />
1 parent 113f01a commit 6007a4d

File tree

3 files changed

+81
-13
lines changed

3 files changed

+81
-13
lines changed

lldb/tools/lldb-dap/package-lock.json

Lines changed: 31 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

lldb/tools/lldb-dap/package.json

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,9 @@
2727
"categories": [
2828
"Debuggers"
2929
],
30+
"dependencies": {
31+
"chokidar": "^4.0.3"
32+
},
3033
"devDependencies": {
3134
"@types/node": "^18.19.41",
3235
"@types/tabulator-tables": "^6.2.10",

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

Lines changed: 47 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,6 @@
1+
import { FSWatcher, watch as chokidarWatch } from 'chokidar';
12
import * as child_process from "node:child_process";
3+
import * as path from "path";
24
import { isDeepStrictEqual } from "util";
35
import * as vscode from "vscode";
46

@@ -12,6 +14,10 @@ export class LLDBDapServer implements vscode.Disposable {
1214
private serverProcess?: child_process.ChildProcessWithoutNullStreams;
1315
private serverInfo?: Promise<{ host: string; port: number }>;
1416
private serverSpawnInfo?: string[];
17+
// Detects changes to the lldb-dap executable file since the server's startup.
18+
private serverFileWatcher?: FSWatcher;
19+
// Indicates whether the lldb-dap executable file has changed since the server's startup.
20+
private serverFileChanged?: boolean;
1521

1622
constructor() {
1723
vscode.commands.registerCommand(
@@ -83,6 +89,11 @@ export class LLDBDapServer implements vscode.Disposable {
8389
});
8490
this.serverProcess = process;
8591
this.serverSpawnInfo = this.getSpawnInfo(dapPath, dapArgs, options?.env);
92+
this.serverFileChanged = false;
93+
this.serverFileWatcher = chokidarWatch(dapPath);
94+
this.serverFileWatcher
95+
.on('change', () => this.serverFileChanged = true)
96+
.on('unlink', () => this.serverFileChanged = true);
8697
});
8798
return this.serverInfo;
8899
}
@@ -100,39 +111,58 @@ export class LLDBDapServer implements vscode.Disposable {
100111
args: string[],
101112
env: NodeJS.ProcessEnv | { [key: string]: string } | undefined,
102113
): Promise<boolean> {
103-
if (!this.serverProcess || !this.serverInfo || !this.serverSpawnInfo) {
114+
if (
115+
!this.serverProcess ||
116+
!this.serverInfo ||
117+
!this.serverSpawnInfo ||
118+
!this.serverFileWatcher ||
119+
this.serverFileChanged === undefined
120+
) {
104121
return true;
105122
}
106123

107-
const newSpawnInfo = this.getSpawnInfo(dapPath, args, env);
108-
if (isDeepStrictEqual(this.serverSpawnInfo, newSpawnInfo)) {
109-
return true;
110-
}
124+
const changeTLDR = [];
125+
const changeDetails = [];
111126

112-
const userInput = await vscode.window.showInformationMessage(
113-
"The arguments to lldb-dap have changed. Would you like to restart the server?",
114-
{
115-
modal: true,
116-
detail: `An existing lldb-dap server (${this.serverProcess.pid}) is running with different arguments.
127+
if (this.serverFileChanged) {
128+
changeTLDR.push("an old binary");
129+
}
117130

131+
const newSpawnInfo = this.getSpawnInfo(dapPath, args, env);
132+
if (!isDeepStrictEqual(this.serverSpawnInfo, newSpawnInfo)) {
133+
changeTLDR.push("different arguments");
134+
changeDetails.push(`
118135
The previous lldb-dap server was started with:
119136
120137
${this.serverSpawnInfo.join(" ")}
121138
122139
The new lldb-dap server will be started with:
123140
124141
${newSpawnInfo.join(" ")}
142+
`
143+
);
144+
}
145+
146+
// If the server hasn't changed, continue startup without killing it.
147+
if (changeTLDR.length === 0) {
148+
return true;
149+
}
125150

151+
// The server has changed. Prompt the user to restart it.
152+
const userInput = await vscode.window.showInformationMessage(
153+
"The lldb-dap server has changed. Would you like to restart the server?",
154+
{
155+
modal: true,
156+
detail: `An existing lldb-dap server (${this.serverProcess.pid}) is running with ${changeTLDR.map(s => `*${s}*`).join(" and ")}.
157+
${changeDetails.join("\n")}
126158
Restarting the server will interrupt any existing debug sessions and start a new server.`,
127159
},
128160
"Restart",
129161
"Use Existing",
130162
);
131163
switch (userInput) {
132164
case "Restart":
133-
this.serverProcess.kill();
134-
this.serverProcess = undefined;
135-
this.serverInfo = undefined;
165+
this.dispose();
136166
return true;
137167
case "Use Existing":
138168
return true;
@@ -156,6 +186,10 @@ Restarting the server will interrupt any existing debug sessions and start a new
156186
if (this.serverProcess === process) {
157187
this.serverProcess = undefined;
158188
this.serverInfo = undefined;
189+
this.serverSpawnInfo = undefined;
190+
this.serverFileWatcher?.close();
191+
this.serverFileWatcher = undefined;
192+
this.serverFileChanged = undefined;
159193
}
160194
}
161195

0 commit comments

Comments
 (0)