Skip to content

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

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.

Sign up for GitHub

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

Closed
wants to merge 2 commits into from
Closed

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
bb9541a
[vscode-lldb] Restart server when lldb-dap's modification time has ch…
royitaqi Sep 17, 2025
4b989d8
Revert unintended changes
royitaqi Sep 18, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
66 changes: 66 additions & 0 deletions lldb/tools/lldb-dap/package-lock.json
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 4 additions & 0 deletions lldb/tools/lldb-dap/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,11 @@
"categories": [
"Debuggers"
],
"dependencies": {
"fs-extra": "^11.0.4"
},
"devDependencies": {
"@types/fs-extra": "^11.0.4",
"@types/node": "^18.19.41",
"@types/tabulator-tables": "^6.2.10",
"@types/vscode": "1.75.0",
Expand Down
31 changes: 22 additions & 9 deletions lldb/tools/lldb-dap/src-ts/lldb-dap-server.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import * as fs from 'fs-extra';
import * as child_process from "node:child_process";
import { isDeepStrictEqual } from "util";
import * as vscode from "vscode";
Expand Down Expand Up @@ -54,7 +55,7 @@ export class LLDBDapServer implements vscode.Disposable {
return this.serverInfo;
}

this.serverInfo = new Promise((resolve, reject) => {
this.serverInfo = new Promise(async (resolve, reject) => {
const process = child_process.spawn(dapPath, dapArgs, options);
process.on("error", (error) => {
reject(error);
Expand Down Expand Up @@ -82,7 +83,7 @@ export class LLDBDapServer implements vscode.Disposable {
}
});
this.serverProcess = process;
this.serverSpawnInfo = this.getSpawnInfo(dapPath, dapArgs, options?.env);
this.serverSpawnInfo = await this.getSpawnInfo(dapPath, dapArgs, options?.env);
});
return this.serverInfo;
}
Expand All @@ -104,13 +105,13 @@ export class LLDBDapServer implements vscode.Disposable {
return true;
}

const newSpawnInfo = this.getSpawnInfo(dapPath, args, env);
const newSpawnInfo = await this.getSpawnInfo(dapPath, args, env);
if (isDeepStrictEqual(this.serverSpawnInfo, newSpawnInfo)) {
return true;
}

const userInput = await vscode.window.showInformationMessage(
"The arguments to lldb-dap have changed. Would you like to restart the server?",
"The lldb-dap binary and/or the arguments to it 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.
Expand All @@ -131,8 +132,7 @@ Restarting the server will interrupt any existing debug sessions and start a new
switch (userInput) {
case "Restart":
this.serverProcess.kill();
this.serverProcess = undefined;
this.serverInfo = undefined;
this.cleanUp(this.serverProcess);
return true;
case "Use Existing":
return true;
Expand All @@ -149,27 +149,40 @@ Restarting the server will interrupt any existing debug sessions and start a new
this.cleanUp(this.serverProcess);
}

cleanUp(process: child_process.ChildProcessWithoutNullStreams) {
private cleanUp(process: child_process.ChildProcessWithoutNullStreams) {
// If the following don't equal, then the fields have already been updated
// (either a new process has started, or the fields were already cleaned
// up), and so the cleanup should be skipped.
if (this.serverProcess === process) {
this.serverProcess = undefined;
this.serverInfo = undefined;
this.serverSpawnInfo = undefined;
}
}

getSpawnInfo(
private async getSpawnInfo(
path: string,
args: string[],
env: NodeJS.ProcessEnv | { [key: string]: string } | undefined,
): string[] {
): Promise<string[]> {
return [
path,
...args,
...Object.entries(env ?? {}).map(
(entry) => String(entry[0]) + "=" + String(entry[1]),
),
`(${await this.getFileModifiedTimestamp(path)})`,
];
}

private async getFileModifiedTimestamp(file: string): Promise<string | null> {
try {
if (!(await fs.pathExists(file))) {
return null;
}
return (await fs.promises.stat(file)).mtime.toLocaleString();
} catch (error) {
return null;
}
}
Comment on lines +177 to +187
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should use the builtin method for this: vscode.workspace.createFileSystemWatcher

that will be more reliable

Copy link
Contributor Author

@royitaqi royitaqi Sep 18, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(The following is probably more for my learning.)

I did a Google search and it said this:

While vscode.createFileSystemWatcher is reliable, file system watching is not always perfect and can be affected by the operating system. According to the VS Code team, the OS may decide to drop file events at any time, so there is no 100% guarantee that an event will be detected.

It seems it's because the underlying fs.watch is "not always reliable".

--

FWIW, I have a local implementation using the createFileSystemWatcher but somehow it's not triggering when I touch the lldb-dap binary. Still trying to figure out why.

The code is very simple:

      this.serverFileWatcher = vscode.workspace.createFileSystemWatcher(absoluteDapPath);
      this.serverFileWatcher.onDidChange(() => {
        this.serverFileChanged = true;
      });

The callback is never executed when I do touch <absoluteDapPath>.

It seems there is a bunch of reasons where createFileSystemWatcher might not work.

--

What reliability issue did you have in mind about the "modification time" approach?

I might have been naive and missed something important. Currently, it appears to me that checking the file modification time is better in terms of simplicity (only depend on the FS supporting modification time), debuggability (user can verify the modification time), and consistency (as long as the modification time changed, it should be detected).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting. I used createFileSystemWatcher for a few extensions that are being shipped in the marketplace already and never got a complaint.
Can you share with me the correct link for a bunch of reasons and "not always reliable".? The links seem to take me to google and not actual pages where you got the info you mention
Also, could you share the snippet of your own local copy of createFileSystemWatcher?

What I believed so far is that createFileSystemWatcher is used internally by vscode to identify when files change, and because it's a crucial feature of the IDE, I assumed it was thoroughly tested in all platforms.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was wrong. What my extensions were using is https://www.npmjs.com/package/chokidar
Try using that one

}
Loading