Skip to content

Commit b3e2b85

Browse files
royitaqiJDevlieghere
authored andcommitted
[vscode-lldb] Fix race condition when changing lldb-dap arguments (llvm#151828)
# 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. (cherry picked from commit bb2642f)
1 parent cdadfab commit b3e2b85

File tree

1 file changed

+13
-6
lines changed

1 file changed

+13
-6
lines changed

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

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -39,8 +39,7 @@ export class LLDBDapServer implements vscode.Disposable {
3939
const process = child_process.spawn(dapPath, dapArgs, options);
4040
process.on("error", (error) => {
4141
reject(error);
42-
this.serverProcess = undefined;
43-
this.serverInfo = undefined;
42+
this.cleanUp(process);
4443
});
4544
process.on("exit", (code, signal) => {
4645
let errorMessage = "Server process exited early";
@@ -50,8 +49,7 @@ export class LLDBDapServer implements vscode.Disposable {
5049
errorMessage += ` due to signal ${signal}`;
5150
}
5251
reject(new Error(errorMessage));
53-
this.serverProcess = undefined;
54-
this.serverInfo = undefined;
52+
this.cleanUp(process);
5553
});
5654
process.stdout.setEncoding("utf8").on("data", (data) => {
5755
const connection = /connection:\/\/\[([^\]]+)\]:(\d+)/.exec(
@@ -126,7 +124,16 @@ Restarting the server will interrupt any existing debug sessions and start a new
126124
return;
127125
}
128126
this.serverProcess.kill();
129-
this.serverProcess = undefined;
130-
this.serverInfo = undefined;
127+
this.cleanUp(this.serverProcess);
128+
}
129+
130+
cleanUp(process: child_process.ChildProcessWithoutNullStreams) {
131+
// If the following don't equal, then the fields have already been updated
132+
// (either a new process has started, or the fields were already cleaned
133+
// up), and so the cleanup should be skipped.
134+
if (this.serverProcess === process) {
135+
this.serverProcess = undefined;
136+
this.serverInfo = undefined;
137+
}
131138
}
132139
}

0 commit comments

Comments
 (0)