[Lldb-commits] [lldb] [vscode-lldb] Restart server when the lldb-dap binary's modification time has changed (PR #159481)

[Roy Shi via lldb-commits]

================
@@ -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;
+    }
+  }
----------------
royitaqi wrote:

(The following is probably more for my learning.)

I did a [Google search](https://www.google.com/search?q=does+vscode.createFileSystemWatcher+detect+changes+from+touch%3F&sca_esv=a1feb94103ceaa7e&ei=IEfMaI7NMZ-i0PEP6orIwQE&ved=0ahUKEwiO3dHC8OKPAxUfETQIHWoFMhgQ4dUDCBA&uact=5&oq=does+vscode.createFileSystemWatcher+detect+changes+from+touch%3F&gs_lp=Egxnd3Mtd2l6LXNlcnAiPmRvZXMgdnNjb2RlLmNyZWF0ZUZpbGVTeXN0ZW1XYXRjaGVyIGRldGVjdCBjaGFuZ2VzIGZyb20gdG91Y2g_SKwhUJIHWKwfcAV4AZABAJgBkwGgAbEKqgEDNS43uAEDyAEA-AEBmAIKoALQBMICChAAGLADGNYEGEfCAgUQABjvBcICCBAAGIAEGKIEmAMA4gMFEgExIECIBgGQBgiSBwM2LjSgB7YosgcDMS40uAfEBMIHBTAuOS4xyAcV&sclient=gws-wiz-serp) 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"](https://www.javascriptcheatsheet.org/cheatsheet/directory-files#:~:text=The%20exact%20behavior%20of%20%60fs.watch%60%20can%20vary%20across%20different%20platforms%20and%20situations.%20It%27s%20not%20always%20reliable%2C%20especially%20on%20network%20file%20systems%20and%20certain%20file%20systems%20like%20NFS%20and%20VFS.%20For%20more%20reliable%20file%20watching%2C%20consider%20using%20a%20library%20like%20%60chokidar%60.).

--

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.

--

Overall, I have no strong opinion. Currently it appears to me that checking the file modification time is better in terms of debuggability (user can verify the modification time).

https://github.com/llvm/llvm-project/pull/159481