[vscode-go] src/goDebugFactory: send SIGINT to delve and avoid treekill
362 views
Skip to first unread message
Hyang-Ah Hana Kim (Gerrit)
Apr 29, 2021, 8:22:14 PM4/29/21
to Suzy Mueller, goph...@pubsubhelper.golang.org, Hyang-Ah Hana Kim, golang-co...@googlegroups.com
Attention is currently required from: Suzy Mueller.
Hyang-Ah Hana Kim would like Suzy Mueller to review this change.
src/goDebugFactory: send SIGINT to delve and avoid treekill
dlv handles SIGINT better than SIGTERM.
And, stop using treekill. Treekill sends signal to dlv and its children
(debugee and their children, and debugserver). It doesn't look like
the debugserver on the mac doesn't seem to finish cleaning up the
target before exiting as a response of SIGINT. The target becomes a
child of the launchd and remains a zombie. Even when debugserver
could handle this gracefully, sending signals to all the children
processes may interfere with debug target's attempt to gracefully
terminate and doesn't seem like a good idea. Trust dlv's termination
handling logic instead.
Testing this beyond manual testing is hard. Instead, just check
whether the temporary binary is removed that is an indirect sign
that delve responded to the signal.
For https://github.com/go-delve/delve/issues/2445
Updates golang/vscode-go#120
Change-Id: I72de903b27b12ed51079d1c21be89c41d8189d8b
---
M src/goDebugFactory.ts
M test/integration/goDebug.test.ts
2 files changed, 49 insertions(+), 2 deletions(-)
diff --git a/src/goDebugFactory.ts b/src/goDebugFactory.ts
index a970121..4b9292b 100644
--- a/src/goDebugFactory.ts
+++ b/src/goDebugFactory.ts
@@ -225,7 +225,7 @@
await new Promise<void>((resolve) => {
const exitTimeoutToken = setTimeout(() => {
this.logger?.error(`dlv dap process (${dlvDapServer.pid}) isn't responding. Killing...`);
- killProcessTree(dlvDapServer);
+ dlvDapServer.kill('SIGINT'); // Don't use treekill but let dlv handle cleaning up the child processes.
resolve();
}, 1_000);
dlvDapServer.on('exit', (code, signal) => {
diff --git a/test/integration/goDebug.test.ts b/test/integration/goDebug.test.ts
index 665eea2..ff86927 100644
--- a/test/integration/goDebug.test.ts
+++ b/test/integration/goDebug.test.ts
@@ -1563,6 +1563,48 @@
await Promise.all([dc.disconnectRequest({ restart: false }), dc.waitForEvent('terminated')]);
});
+
+ test('should cleanup when stopped', async function () {
+ if (!isDlvDap || !dlvDapSkipsEnabled) {
+ this.skip();
+ }
+ const PROGRAM = path.join(DATA_ROOT, 'loop');
+ const OUTPUT = path.join(PROGRAM, '_loop_output');
+
+ const config = {
+ name: 'Launch',
+ type: 'go',
+ request: 'launch',
+ mode: 'debug',
+ program: PROGRAM,
+ stopOnEntry: false,
+ output: OUTPUT
+ };
+ const debugConfig = await initializeDebugConfig(config);
+
+ await Promise.all([dc.configurationSequence(), dc.launch(debugConfig)]);
+
+ try {
+ const fsstat = util.promisify(fs.stat);
+ await fsstat(OUTPUT);
+ } catch (e) {
+ assert.fail(`debug output ${OUTPUT} wasn't built: ${e}`);
+ }
+
+ // It's possible dlv-dap doesn't respond. So, don't wait.
+ dc.disconnectRequest({ restart: false });
+ await sleep(100);
+ dlvDapAdapter.dispose();
+ await sleep(200); // give dlv time to respond to the signal.
+ try {
+ const fsstat = util.promisify(fs.stat);
+ const stat = await fsstat(OUTPUT);
+ fs.unlinkSync(OUTPUT);
+ assert.notStrictEqual(stat, null, `debug output ${OUTPUT} wasn't cleaned up.`);
+ } catch (e) {
+ console.log(`output was cleaned ${OUTPUT} ${e}`);
+ }
+ });
});
suite('switch goroutine', () => {
@@ -1950,9 +1992,10 @@
return;
}
this._disposed = true;
+ this.log('adapter disposing');
+ await super.dispose();
this.log('adapter disposed');
await this._server.close();
- await super.dispose();
}
// Code from
@@ -2023,3 +2066,7 @@
this._log.forEach((msg) => console.log(msg));
}
}
+
+function sleep(ms: number) {
+ return new Promise((resolve) => setTimeout(resolve, ms));
+}
To view, visit change 315151. To unsubscribe, or for help writing mail filters, visit settings.
kokoro (Gerrit)
Apr 29, 2021, 8:32:35 PM4/29/21
to Hyang-Ah Hana Kim, goph...@pubsubhelper.golang.org, Suzy Mueller, golang-co...@googlegroups.com
Attention is currently required from: Suzy Mueller, Hyang-Ah Hana Kim.
Kokoro presubmit build finished with status: FAILURE
Logs at: https://source.cloud.google.com/results/invocations/9c9ebfff-6ffc-431d-8ed1-88bd6ca7cd63
Patch set 1:TryBot-Result -1
Hyang-Ah Hana Kim (Gerrit)
Apr 29, 2021, 8:45:15 PM4/29/21
to Hyang-Ah Hana Kim, goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com
Attention is currently required from: Suzy Mueller, Hyang-Ah Hana Kim.
Hyang-Ah Hana Kim uploaded patch set #2 to this change.
src/goDebugFactory: send SIGINT to delve and avoid treekill
dlv handles SIGINT better than SIGTERM.
And, stop using treekill. Treekill sends signal to dlv and its children
(debugee and their children, and debugserver). It doesn't look like
the debugserver on the mac doesn't seem to finish cleaning up the
target before exiting as a response of SIGINT. The target becomes a
child of the launchd and remains a zombie. Even when debugserver
could handle this gracefully, sending signals to all the children
processes may interfere with debug target's attempt to gracefully
terminate and doesn't seem like a good idea. Trust dlv's termination
handling logic instead.
Testing this beyond manual testing is hard. Instead, just check
whether the temporary binary is removed that is an indirect sign
that delve responded to the signal.
For https://github.com/go-delve/delve/issues/2445
Updates golang/vscode-go#120
Change-Id: I72de903b27b12ed51079d1c21be89c41d8189d8b
---
M src/goDebugFactory.ts
M test/integration/goDebug.test.ts
2 files changed, 49 insertions(+), 2 deletions(-)
To view, visit change 315151. To unsubscribe, or for help writing mail filters, visit settings.
kokoro (Gerrit)
Apr 29, 2021, 8:56:47 PM4/29/21
to Hyang-Ah Hana Kim, goph...@pubsubhelper.golang.org, Suzy Mueller, golang-co...@googlegroups.com
Attention is currently required from: Suzy Mueller, Hyang-Ah Hana Kim.
Kokoro presubmit build finished with status: FAILURE
Logs at: https://source.cloud.google.com/results/invocations/fae84cad-d32d-4d3f-b985-286f51ed775e
Patch set 2:TryBot-Result -1
Hyang-Ah Hana Kim (Gerrit)
Apr 29, 2021, 9:08:36 PM4/29/21
to Hyang-Ah Hana Kim, goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com
Attention is currently required from: Suzy Mueller, Hyang-Ah Hana Kim.
Hyang-Ah Hana Kim uploaded patch set #3 to this change.
src/goDebugFactory: send SIGINT to delve and avoid treekill
dlv handles SIGINT better than SIGTERM.
And, stop using treekill. Treekill sends signal to dlv and its children
(debugee and their children, and debugserver). It doesn't look like
the debugserver on the mac doesn't seem to finish cleaning up the
target before exiting as a response of SIGINT. The target becomes a
child of the launchd and remains a zombie. Even when debugserver
could handle this gracefully, sending signals to all the children
processes may interfere with debug target's attempt to gracefully
terminate and doesn't seem like a good idea. Trust dlv's termination
handling logic instead.
Testing this beyond manual testing is hard. Instead, just check
whether the temporary binary is removed that is an indirect sign
that delve responded to the signal.
For https://github.com/go-delve/delve/issues/2445
Updates golang/vscode-go#120
Change-Id: I72de903b27b12ed51079d1c21be89c41d8189d8b
---
M src/goDebugFactory.ts
M test/integration/goDebug.test.ts
2 files changed, 49 insertions(+), 2 deletions(-)
To view, visit change 315151. To unsubscribe, or for help writing mail filters, visit settings.
kokoro (Gerrit)
Apr 29, 2021, 9:16:30 PM4/29/21
to Hyang-Ah Hana Kim, goph...@pubsubhelper.golang.org, Suzy Mueller, golang-co...@googlegroups.com
Attention is currently required from: Suzy Mueller, Hyang-Ah Hana Kim.
Kokoro presubmit build finished with status: FAILURE
Logs at: https://source.cloud.google.com/results/invocations/ba8d6a06-c98a-431b-ac50-38d65145b83a
Hyang-Ah Hana Kim (Gerrit)
Apr 29, 2021, 9:28:29 PM4/29/21
to Hyang-Ah Hana Kim, goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com
Attention is currently required from: Suzy Mueller, Hyang-Ah Hana Kim.
Hyang-Ah Hana Kim uploaded patch set #4 to this change.
src/goDebugFactory: send SIGINT to delve and avoid treekill
dlv handles SIGINT better than SIGTERM.
And, stop using treekill. Treekill sends signal to dlv and its children
(debugee and their children, and debugserver). It doesn't look like
the debugserver on the mac doesn't seem to finish cleaning up the
target before exiting as a response of SIGINT. The target becomes a
child of the launchd and remains a zombie. Even when debugserver
could handle this gracefully, sending signals to all the children
processes may interfere with debug target's attempt to gracefully
terminate and doesn't seem like a good idea. Trust dlv's termination
handling logic instead.
Testing this beyond manual testing is hard. Instead, just check
whether the temporary binary is removed that is an indirect sign
that delve responded to the signal.
For https://github.com/go-delve/delve/issues/2445
Updates golang/vscode-go#120
Change-Id: I72de903b27b12ed51079d1c21be89c41d8189d8b
---
M src/goDebugFactory.ts
M test/integration/goDebug.test.ts
2 files changed, 52 insertions(+), 3 deletions(-)
To view, visit change 315151. To unsubscribe, or for help writing mail filters, visit settings.
kokoro (Gerrit)
Apr 29, 2021, 9:37:08 PM4/29/21
to Hyang-Ah Hana Kim, goph...@pubsubhelper.golang.org, Suzy Mueller, golang-co...@googlegroups.com
Attention is currently required from: Suzy Mueller, Hyang-Ah Hana Kim.
Kokoro presubmit build finished with status: SUCCESS
Logs at: https://source.cloud.google.com/results/invocations/c41cb1d8-6f8e-4517-83c3-7360cff55491
Patch set 4:TryBot-Result +1
Hyang-Ah Hana Kim (Gerrit)
Apr 29, 2021, 10:20:30 PM4/29/21
to Hyang-Ah Hana Kim, goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com
Attention is currently required from: Suzy Mueller, Hyang-Ah Hana Kim.
Hyang-Ah Hana Kim uploaded patch set #5 to this change.
src/goDebugFactory: send SIGINT to delve and avoid treekill
dlv handles SIGINT better than SIGTERM.
And, stop using treekill. Treekill sends signal to dlv and its children
(debugee and their children, and debugserver). It doesn't look like
the debugserver on the mac doesn't seem to finish cleaning up the
target before exiting as a response of SIGINT. The target becomes a
child of the launchd and remains a zombie. Even when debugserver
could handle this gracefully, sending signals to all the children
processes may interfere with debug target's attempt to gracefully
terminate and doesn't seem like a good idea. Trust dlv's termination
handling logic instead.
Testing this beyond manual testing is hard. Instead, just check
whether the temporary binary is removed that is an indirect sign
that delve responded to the signal.
For https://github.com/go-delve/delve/issues/2445
Updates golang/vscode-go#120
Change-Id: I72de903b27b12ed51079d1c21be89c41d8189d8b
---
M src/goDebugFactory.ts
M test/integration/goDebug.test.ts
2 files changed, 52 insertions(+), 3 deletions(-)
To view, visit change 315151. To unsubscribe, or for help writing mail filters, visit settings.
kokoro (Gerrit)
Apr 29, 2021, 10:32:09 PM4/29/21
to Hyang-Ah Hana Kim, goph...@pubsubhelper.golang.org, Suzy Mueller, golang-co...@googlegroups.com
Attention is currently required from: Suzy Mueller, Hyang-Ah Hana Kim.
Kokoro presubmit build finished with status: SUCCESS
Logs at: https://source.cloud.google.com/results/invocations/c009bc65-0262-483b-acbf-c79ac4d8a9bc
Hyang-Ah Hana Kim (Gerrit)
Apr 29, 2021, 10:38:10 PM4/29/21
to Hyang-Ah Hana Kim, goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com
Attention is currently required from: Suzy Mueller, Hyang-Ah Hana Kim.
Hyang-Ah Hana Kim uploaded patch set #6 to this change.
src/goDebugFactory: send SIGINT to delve and avoid treekill
dlv handles SIGINT better than SIGTERM.
And, stop using treekill. Treekill sends signal to dlv and its children
(debugee and their children, and debugserver). It doesn't look like
the debugserver on the mac doesn't seem to finish cleaning up the
target before exiting as a response of SIGINT. The target becomes a
child of the launchd and remains a zombie. Even when debugserver
could handle this gracefully, sending signals to all the children
processes may interfere with debug target's attempt to gracefully
terminate and doesn't seem like a good idea. Trust dlv's termination
handling logic instead.
Testing this beyond manual testing is hard. Instead, just check
whether the temporary binary is removed that is an indirect sign
that delve responded to the signal.
For https://github.com/go-delve/delve/issues/2445
Updates golang/vscode-go#120
Change-Id: I72de903b27b12ed51079d1c21be89c41d8189d8b
---
M src/goDebugFactory.ts
M test/integration/goDebug.test.ts
2 files changed, 52 insertions(+), 3 deletions(-)
To view, visit change 315151. To unsubscribe, or for help writing mail filters, visit settings.
Hyang-Ah Hana Kim (Gerrit)
Apr 29, 2021, 10:41:44 PM4/29/21
to Hyang-Ah Hana Kim, goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com
Attention is currently required from: Suzy Mueller, Hyang-Ah Hana Kim.
Hyang-Ah Hana Kim uploaded patch set #7 to this change.
src/goDebugFactory: send SIGINT to delve and avoid treekill
dlv handles SIGINT better than SIGTERM.
And, stop using treekill. Treekill sends signal to dlv and its children
(debugee and their children, and debugserver). It doesn't look like
the debugserver on the mac doesn't seem to finish cleaning up the
target before exiting as a response of SIGINT. The target becomes a
child of the launchd and remains a zombie. Even when debugserver
could handle this gracefully, sending signals to all the children
processes may interfere with debug target's attempt to gracefully
terminate and doesn't seem like a good idea. Trust dlv's termination
handling logic instead.
Testing this beyond manual testing is hard. Instead, just check
whether the temporary binary is removed that is an indirect sign
that delve responded to the signal.
For https://github.com/go-delve/delve/issues/2445
Updates golang/vscode-go#120
Change-Id: I72de903b27b12ed51079d1c21be89c41d8189d8b
---
M src/goDebugFactory.ts
M test/integration/goDebug.test.ts
M test/testdata/loop/loop.go
3 files changed, 54 insertions(+), 5 deletions(-)
To view, visit change 315151. To unsubscribe, or for help writing mail filters, visit settings.
kokoro (Gerrit)
Apr 29, 2021, 10:46:26 PM4/29/21
to Hyang-Ah Hana Kim, goph...@pubsubhelper.golang.org, Suzy Mueller, golang-co...@googlegroups.com
Attention is currently required from: Suzy Mueller, Hyang-Ah Hana Kim.
Kokoro presubmit build finished with status: FAILURE
Logs at: https://source.cloud.google.com/results/invocations/fd777342-66ee-4491-b250-c6757b2714ad
Patch set 6:TryBot-Result -1
kokoro (Gerrit)
Apr 29, 2021, 10:51:32 PM4/29/21
to Hyang-Ah Hana Kim, goph...@pubsubhelper.golang.org, Suzy Mueller, golang-co...@googlegroups.com
Attention is currently required from: Suzy Mueller, Hyang-Ah Hana Kim.
Kokoro presubmit build finished with status: FAILURE
Logs at: https://source.cloud.google.com/results/invocations/6459e8b8-b3a8-4ca3-b0df-8f36177de20f
Patch set 7:TryBot-Result -1
Hyang-Ah Hana Kim (Gerrit)
Apr 29, 2021, 11:17:06 PM4/29/21
to Hyang-Ah Hana Kim, goph...@pubsubhelper.golang.org, kokoro, Suzy Mueller, golang-co...@googlegroups.com
Attention is currently required from: Suzy Mueller.
Patch set 8:-Run-TryBot
Hyang-Ah Hana Kim (Gerrit)
Apr 29, 2021, 11:17:20 PM4/29/21
to Hyang-Ah Hana Kim, goph...@pubsubhelper.golang.org, kokoro, Suzy Mueller, golang-co...@googlegroups.com
Attention is currently required from: Suzy Mueller.
Patch set 8:Run-TryBot +1
Hyang-Ah Hana Kim (Gerrit)
Apr 29, 2021, 11:17:33 PM4/29/21
to Hyang-Ah Hana Kim, goph...@pubsubhelper.golang.org, Suzy Mueller, kokoro, golang-co...@googlegroups.com
Hyang-Ah Hana Kim removed Suzy Mueller from this change.
To view, visit change 315151. To unsubscribe, or for help writing mail filters, visit settings.
kokoro (Gerrit)
Apr 29, 2021, 11:26:42 PM4/29/21
to Hyang-Ah Hana Kim, goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com
Attention is currently required from: Hyang-Ah Hana Kim.
Kokoro presubmit build finished with status: FAILURE
Logs at: https://source.cloud.google.com/results/invocations/9912f2c2-28cf-4c7f-9625-ce3a4a578de8
Patch set 8:TryBot-Result -1
Hyang-Ah Hana Kim (Gerrit)
Apr 30, 2021, 10:40:01 AM4/30/21
to Hyang-Ah Hana Kim, goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com
Attention is currently required from: Hyang-Ah Hana Kim.
Hyang-Ah Hana Kim uploaded patch set #9 to this change.
src/goDebugFactory: send SIGINT to delve and avoid treekill
dlv handles SIGINT better than SIGTERM.
And, stop using treekill. Treekill sends signal to dlv and its children
(debugee and their children, and debugserver). It doesn't look like
the debugserver on the mac doesn't seem to finish cleaning up the
target before exiting as a response of SIGINT. The target becomes a
child of the launchd and remains a zombie. Even when debugserver
could handle this gracefully, sending signals to all the children
processes may interfere with debug target's attempt to gracefully
terminate and doesn't seem like a good idea. Trust dlv's termination
handling logic instead.
Testing this beyond manual testing is hard. Instead, just check
whether the temporary binary is removed that is an indirect sign
that delve responded to the signal.
And fix some places in goDebugFactory where null logger object
can cause to crash.
For https://github.com/go-delve/delve/issues/2445
Updates golang/vscode-go#120
Change-Id: I72de903b27b12ed51079d1c21be89c41d8189d8b
---
M src/goDebugFactory.ts
M test/integration/goDebug.test.ts
M test/testdata/loop/loop.go
3 files changed, 66 insertions(+), 8 deletions(-)
To view, visit change 315151. To unsubscribe, or for help writing mail filters, visit settings.
kokoro (Gerrit)
Apr 30, 2021, 10:52:07 AM4/30/21
to Hyang-Ah Hana Kim, goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com
Attention is currently required from: Hyang-Ah Hana Kim.
Kokoro presubmit build finished with status: SUCCESS
Logs at: https://source.cloud.google.com/results/invocations/5a2fcdeb-9bcd-444c-b3a4-63eda72a0698
Patch set 9:TryBot-Result +1
Hyang-Ah Hana Kim (Gerrit)
Apr 30, 2021, 11:31:59 AM4/30/21
to Hyang-Ah Hana Kim, goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com
Attention is currently required from: Hyang-Ah Hana Kim.
Hyang-Ah Hana Kim uploaded patch set #10 to this change.
src/goDebugFactory: send SIGINT to delve and avoid treekill
dlv handles SIGINT better than SIGTERM.
And, stop using treekill. Treekill sends signal to dlv and its children
(debugee and their children, and debugserver). It doesn't look like
the debugserver on the mac doesn't seem to finish cleaning up the
target before exiting as a response of SIGINT. The target becomes a
child of the launchd and remains a zombie. Even when debugserver
could handle this gracefully, sending signals to all the children
processes may interfere with debug target's attempt to gracefully
terminate and doesn't seem like a good idea. Trust dlv's termination
handling logic instead.
Testing this beyond manual testing is hard. Instead, just check
whether the temporary binary is removed that is an indirect sign
that delve responded to the signal.
Changed DelveDAPOutputAdapter's dispose to take an optional timeout
parameter - to use it in testing.
And this CL fixes some places in goDebugFactory where null logger object
can cause to crash.
For https://github.com/go-delve/delve/issues/2445
Updates golang/vscode-go#120
Change-Id: I72de903b27b12ed51079d1c21be89c41d8189d8b
---
M src/goDebugFactory.ts
M test/integration/goDebug.test.ts
M test/testdata/loop/loop.go
3 files changed, 67 insertions(+), 12 deletions(-)
To view, visit change 315151. To unsubscribe, or for help writing mail filters, visit settings.
kokoro (Gerrit)
Apr 30, 2021, 11:42:33 AM4/30/21
to Hyang-Ah Hana Kim, goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com
Attention is currently required from: Hyang-Ah Hana Kim.
Kokoro presubmit build finished with status: SUCCESS
Logs at: https://source.cloud.google.com/results/invocations/82637e35-6590-4e18-a5d2-493ef0cbd26e
Patch set 10:TryBot-Result +1
Suzy Mueller (Gerrit)
Apr 30, 2021, 11:50:45 AM4/30/21
to Hyang-Ah Hana Kim, goph...@pubsubhelper.golang.org, Polina Sokolova, kokoro, golang-co...@googlegroups.com
Attention is currently required from: Hyang-Ah Hana Kim.
Patch set 10:Code-Review +2Trust +1
1 comment:
Patchset:
Thanks for doing this and adding a test!
To view, visit change 315151. To unsubscribe, or for help writing mail filters, visit settings.
Hyang-Ah Hana Kim (Gerrit)
Apr 30, 2021, 12:03:08 PM4/30/21
to Hyang-Ah Hana Kim, goph...@pubsubhelper.golang.org, golang-...@googlegroups.com, Suzy Mueller, Polina Sokolova, kokoro, golang-co...@googlegroups.com
Hyang-Ah Hana Kim submitted this change.
Approvals:
Suzy Mueller: Looks good to me, approved; Trusted
Hyang-Ah Hana Kim: Trusted; Run TryBots
kokoro: TryBots succeeded
src/goDebugFactory: send SIGINT to delve and avoid treekill
dlv handles SIGINT better than SIGTERM.
And, stop using treekill. Treekill sends signal to dlv and its children
(debugee and their children, and debugserver). It doesn't look like
the debugserver on the mac doesn't seem to finish cleaning up the
target before exiting as a response of SIGINT. The target becomes a
child of the launchd and remains a zombie. Even when debugserver
could handle this gracefully, sending signals to all the children
processes may interfere with debug target's attempt to gracefully
terminate and doesn't seem like a good idea. Trust dlv's termination
handling logic instead.
Testing this beyond manual testing is hard. Instead, just check
whether the temporary binary is removed that is an indirect sign
that delve responded to the signal.
Changed DelveDAPOutputAdapter's dispose to take an optional timeout
parameter - to use it in testing.
And this CL fixes some places in goDebugFactory where null logger object
can cause to crash.
For https://github.com/go-delve/delve/issues/2445
Updates golang/vscode-go#120
Change-Id: I72de903b27b12ed51079d1c21be89c41d8189d8b
Reviewed-on: https://go-review.googlesource.com/c/vscode-go/+/315151
Trust: Hyang-Ah Hana Kim <hya...@gmail.com>
Trust: Suzy Mueller <suz...@golang.org>
Run-TryBot: Hyang-Ah Hana Kim <hya...@gmail.com>
TryBot-Result: kokoro <noreply...@google.com>
Reviewed-by: Suzy Mueller <suz...@golang.org>
---
M src/goDebugFactory.ts
M test/integration/goDebug.test.ts
M test/testdata/loop/loop.go
3 files changed, 67 insertions(+), 12 deletions(-)
diff --git a/src/goDebugFactory.ts b/src/goDebugFactory.ts
index a970121..720307e 100644
--- a/src/goDebugFactory.ts
+++ b/src/goDebugFactory.ts
@@ -106,13 +106,13 @@
'utf8',
(err) => {
if (err) {
- this.logger.error(`error sending message: ${err}`);
+ this.logger?.error(`error sending message: ${err}`);
this.sendMessageToClient(new TerminatedEvent());
}
}
);
} else {
- this.logger.error(`stream is closed; dropping ${json}`);
+ this.logger?.error(`stream is closed; dropping ${json}`);
}
}
@@ -135,7 +135,7 @@
this.terminated = true;
if (err) {
- this.logger.error(`connection error: ${err}`);
+ this.logger?.error(`connection error: ${err}`);
this.sendMessageToClient(new OutputEvent(`connection error: ${err}\n`, 'console'));
}
this.sendMessageToClient(new TerminatedEvent());
@@ -204,7 +204,7 @@
super.sendMessageToServer(message);
}
- async dispose() {
+ async dispose(timeoutMS?: number) {
// NOTE: OutputEvents from here may not show up in DEBUG CONSOLE
// because the debug session is terminating.
await super.dispose();
@@ -212,7 +212,11 @@
return;
}
+ if (timeoutMS === undefined) {
+ timeoutMS = 1_000;
+ }
const dlvDapServer = this.dlvDapServer;
+ this.dlvDapServer = undefined;
if (!dlvDapServer) {
return;
}
@@ -225,9 +229,9 @@
await new Promise<void>((resolve) => {
const exitTimeoutToken = setTimeout(() => {
this.logger?.error(`dlv dap process (${dlvDapServer.pid}) isn't responding. Killing...`);
- killProcessTree(dlvDapServer);
+ dlvDapServer.kill('SIGINT'); // Don't use treekill but let dlv handle cleaning up the child processes.
resolve();
- }, 1_000);
+ }, timeoutMS);
dlvDapServer.on('exit', (code, signal) => {
clearTimeout(exitTimeoutToken);
this.logger?.error(
diff --git a/test/integration/goDebug.test.ts b/test/integration/goDebug.test.ts
index 665eea2..8628162 100644
--- a/test/integration/goDebug.test.ts
+++ b/test/integration/goDebug.test.ts
@@ -340,7 +340,7 @@
}
d.dispose();
} else {
- dc.stop();
+ dc?.stop();
}
sinon.restore();
});
@@ -1563,6 +1563,52 @@
+ await sleep(10);
+ dlvDapAdapter.dispose(1);
+ dlvDapAdapter = undefined;
+ dc = undefined;
+ await sleep(100); // allow dlv to respond and finish cleanup.
+ let stat: fs.Stats = null;
+ try {
+ const fsstat = util.promisify(fs.stat);
+ stat = await fsstat(OUTPUT);
+ fs.unlinkSync(OUTPUT);
+ } catch (e) {
+ console.log(`output was cleaned ${OUTPUT} ${e}`);
+ }
+ assert.strictEqual(stat, null, `debug output ${OUTPUT} wasn't cleaned up. ${JSON.stringify(stat)}`);
+ console.log('finished');
+ });
});
suite('switch goroutine', () => {
@@ -1945,14 +1991,15 @@
}
private _disposed = false;
- public async dispose() {
+ public async dispose(timeoutMS?: number) {
if (this._disposed) {
return;
}
this._disposed = true;
- this.log('adapter disposed');
+ this.log('adapter disposing');
await this._server.close();
- await super.dispose();
+ await super.dispose(timeoutMS);
+ this.log('adapter disposed');
}
// Code from
@@ -2023,3 +2070,7 @@
this._log.forEach((msg) => console.log(msg));
}
}
+
+function sleep(ms: number) {
+ return new Promise((resolve) => setTimeout(resolve, ms));
+}
diff --git a/test/testdata/loop/loop.go b/test/testdata/loop/loop.go
index 9740987..394e063 100644
--- a/test/testdata/loop/loop.go
+++ b/test/testdata/loop/loop.go
@@ -1,7 +1,7 @@
package main
-
+import "time"
func main() {
for {
- print("Hello")
+ time.Sleep(10*time.Millisecond)
}
}
To view, visit change 315151. To unsubscribe, or for help writing mail filters, visit settings.
Reply all
Reply to author
Forward
0 new messages