[vscode-lldb] Fix race condition when changing lldb-dap arguments

[royitaqi]

Problem

When the user changes lldb-dap's arguments (e.g. path), there is a race condition, where the new lldb-dap process could be started first and have set the extension's serverProcess and serverInfo according to the new process, while the old lldb-dap process exits later and wipes out these two fields.

Consequences:

1. This causes getServerProcess() to return undefined when it should return the new process.
2. This also causes wrong behavior when starting the next debug session that a new lldb-dap process will be started and the old not reused nor killed.

Fix

When wiping the two fields, check if serverProcess equals to the process captured by the handler. If they equal, wipe the fields. If not, then the fields have already been updated (either new process has started, or the fields were already wiped out by another handler), and so the wiping should be skipped.

[llvmbot]

@llvm/pr-subscribers-lldb

Author: None (royitaqi)

Changes

Problem

When the user changes lldb-dap's arguments (e.g. path), there is a race condition, where the new lldb-dap process could be started first and have set the extension's serverProcess and serverInfo according to the new process, while the old lldb-dap process exits later and wipes out these two fields.

Consequences:

1. This causes getServerProcess() to return undefined when it should return the new process.
2. This also causes wrong behavior in the next debug session that a new lldb-dap process will be started and the old not reused nor killed.

Fix

When wiping the two fields, check if serverProcess equals to the process captured by the handler. If they equal, wipe the fields. If not, then the fields have already been updated (either new process has started, or the fields were already wiped out by another handler), and so the wiping should be skipped.

---

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

1 Files Affected:

• (modified) lldb/tools/lldb-dap/src-ts/lldb-dap-server.ts (+8-4)

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 79573ec7342b1..c4e99a3178e8c 100644
--- a/lldb/tools/lldb-dap/src-ts/lldb-dap-server.ts
+++ b/lldb/tools/lldb-dap/src-ts/lldb-dap-server.ts
@@ -39,8 +39,10 @@ export class LLDBDapServer implements vscode.Disposable {
       const process = child_process.spawn(dapPath, dapArgs, options);
       process.on("error", (error) => {
         reject(error);
-        this.serverProcess = undefined;
-        this.serverInfo = undefined;
+        if (this.serverProcess === process) {
+          this.serverProcess = undefined;
+          this.serverInfo = undefined;
+        }
       });
       process.on("exit", (code, signal) => {
         let errorMessage = "Server process exited early";
@@ -50,8 +52,10 @@ export class LLDBDapServer implements vscode.Disposable {
           errorMessage += ` due to signal ${signal}`;
         }
         reject(new Error(errorMessage));
-        this.serverProcess = undefined;
-        this.serverInfo = undefined;
+        if (this.serverProcess === process) {
+          this.serverProcess = undefined;
+          this.serverInfo = undefined;
+        }
       });
       process.stdout.setEncoding("utf8").on("data", (data) => {
         const connection = /connection:\/\/\[([^\]]+)\]:(\d+)/.exec(

[royitaqi]

Updated. @walter-erquinigo kindly take a look when you have the time.
I appreciate your review~!

[walter-erquinigo]

thanks!