Skip to content

Commit 3ea718c

Browse files
Copilotrchiodo
andauthored
Fix premature test session exit when child process debug session terminates
Agent-Logs-Url: https://github.com/microsoft/vscode-python-debugger/sessions/c5ea3960-ac08-42e5-8c37-fa964ce38c9e Co-authored-by: rchiodo <19672699+rchiodo@users.noreply.github.com>
1 parent d7c3ead commit 3ea718c

6 files changed

Lines changed: 192 additions & 3 deletions

File tree

package-lock.json

Lines changed: 2 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

src/extension/debugger/hooks/childProcessAttachService.ts

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,18 @@ import { traceLog } from '../../common/log/logging';
2222
export class ChildProcessAttachService implements IChildProcessAttachService {
2323
@captureTelemetry(EventName.DEBUGGER_ATTACH_TO_CHILD_PROCESS)
2424
public async attach(data: AttachRequestArguments & DebugConfiguration, parentSession: DebugSession): Promise<void> {
25-
const debugConfig: AttachRequestArguments & DebugConfiguration = data;
25+
const debugConfig: AttachRequestArguments & DebugConfiguration = { ...data };
26+
27+
// Remove the 'purpose' field from the child process debug configuration.
28+
// The child session inherits the parent's configuration (including 'purpose')
29+
// via debugpy's notify_of_subprocess. If the parent is a test debug session
30+
// (purpose: ['debug-test']), the child would also appear to be a test session.
31+
// This can cause the Python extension's test adapter to incorrectly treat the
32+
// child process session termination as the end of the test run, which results
33+
// in premature disconnection of the parent (test runner) debug session.
34+
// See: https://github.com/microsoft/vscode-python-debugger/issues/548
35+
delete debugConfig.purpose;
36+
2637
const debugSessionOption: DebugSessionOptions = {
2738
parentSession: parentSession,
2839
lifecycleManagedByParent: true,

src/test/.vscode/launch.json

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
{
2+
"version": "0.2.0",
3+
"configurations": [
4+
{
5+
"name": "launch a file",
6+
"type": "debugpy",
7+
"request": "launch",
8+
"program": "${file}",
9+
"args": [],
10+
"console": "integratedTerminal",
11+
"justMyCode": false
12+
},
13+
{
14+
"name": "attach to a local port",
15+
"type": "debugpy",
16+
"request": "attach",
17+
"port": 5678,
18+
"host": "localhost",
19+
"pathMappings": [
20+
{
21+
"localRoot": "${workspaceFolder}",
22+
"remoteRoot": "."
23+
}
24+
],
25+
"justMyCode": false
26+
},
27+
{
28+
"name": "attach to a local PID",
29+
"type": "debugpy",
30+
"request": "attach",
31+
"processId": "${command:pickProcess}",
32+
"justMyCode": false
33+
}
34+
]
35+
}
Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
"""
2+
Pytest test file demonstrating the multiprocessing issue when debugging.
3+
4+
When debugging pytest tests, creating a child process via multiprocessing.Process
5+
and then waiting for it via process.join() can cause premature exit of the debug
6+
session. Specifically, the debugger terminates immediately after the child process
7+
terminates (i.e., at the point where process.join() returns), before reaching
8+
subsequent statements in the test body.
9+
10+
This file contains tests that reproduce the issue described in:
11+
https://github.com/microsoft/vscode-python-debugger/issues/548
12+
13+
Workaround: Use a launch.json with "-s" (--capture=no) in pytest args, or
14+
launch pytest via a custom launch.json with "module": "pytest".
15+
"""
16+
import multiprocessing as mp
17+
import time
18+
19+
20+
def _child_main() -> None:
21+
"""Simple child process that sleeps briefly then exits."""
22+
time.sleep(0.25)
23+
24+
25+
def test_multiprocessing_join_in_test_body() -> None:
26+
"""
27+
Regression test: process.join() in a pytest test body should not cause
28+
premature debugger exit.
29+
30+
When debugging this test, the debugger should reach print("after!") and
31+
the assertion, not exit immediately after process.join() returns.
32+
"""
33+
proc = mp.Process(
34+
target=_child_main,
35+
name="debug_multiprocessing_test_body_child",
36+
)
37+
proc.daemon = True
38+
proc.start()
39+
print("before!")
40+
proc.join(10)
41+
print("after!")
42+
assert proc.exitcode == 0
43+
44+
45+
def test_multiprocessing_nondaemon_join() -> None:
46+
"""
47+
Variant: non-daemon child process with join().
48+
49+
Should behave the same as the daemon variant when debugging.
50+
"""
51+
proc = mp.Process(
52+
target=_child_main,
53+
name="debug_multiprocessing_nondaemon_child",
54+
)
55+
proc.start()
56+
proc.join(10)
57+
assert proc.exitcode == 0
Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
"""
2+
Wait for a file to appear, then print 'done!' and exit.
3+
4+
This script is used to keep a debug session alive (by pausing the script
5+
until the test harness is ready to terminate it), and to produce a known
6+
output (useful for verifying that output was captured correctly).
7+
8+
Usage:
9+
python wait_for_file.py <done_file> [output_file]
10+
"""
11+
import os
12+
import sys
13+
import time
14+
15+
16+
def wait_for_file(path: str) -> None:
17+
while not os.path.exists(path):
18+
time.sleep(0.1)
19+
20+
21+
def main() -> None:
22+
args = sys.argv[1:]
23+
if not args:
24+
print("usage: wait_for_file.py <done_file> [output_file]", file=sys.stderr)
25+
sys.exit(1)
26+
27+
done_file = args[0]
28+
output_file = args[1] if len(args) > 1 else None
29+
30+
wait_for_file(done_file)
31+
32+
if output_file:
33+
with open(output_file, "w") as f:
34+
f.write("done!\n")
35+
else:
36+
print("done!")
37+
38+
39+
if __name__ == "__main__":
40+
main()

src/test/unittest/hooks/childProcessAttachService.unit.test.ts

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -189,4 +189,50 @@ suite('Debug - Attach to Child Process', () => {
189189
expect(thirdArg).to.deep.equal({ parentSession: session, lifecycleManagedByParent: true });
190190
sinon.assert.notCalled(showErrorMessageStub);
191191
});
192+
test('Child process debug config should not inherit purpose from parent session', async () => {
193+
// When the parent session is a test debug session (purpose: ['debug-test']),
194+
// the child process config inherits 'purpose' via debugpy's notify_of_subprocess.
195+
// We must strip 'purpose' from the child config so that VS Code's test adapter
196+
// does not treat child process session termination as test run completion,
197+
// which would cause premature disconnection of the parent debug session.
198+
// Regression test for: https://github.com/microsoft/vscode-python-debugger/issues/548
199+
const data: AttachRequestArguments = {
200+
request: 'attach',
201+
type: debuggerTypeName,
202+
name: 'Attach',
203+
port: 1234,
204+
subProcessId: 2,
205+
purpose: ['debug-test'],
206+
};
207+
208+
const session: any = {};
209+
getWorkspaceFoldersStub.returns(undefined);
210+
startDebuggingStub.resolves(true);
211+
212+
await attachService.attach(data, session);
213+
214+
sinon.assert.calledOnce(startDebuggingStub);
215+
const [, secondArg] = startDebuggingStub.args[0];
216+
expect(secondArg).to.not.have.property('purpose');
217+
sinon.assert.notCalled(showErrorMessageStub);
218+
});
219+
test('Attaching to child process does not mutate the original data object', async () => {
220+
const data: AttachRequestArguments = {
221+
request: 'attach',
222+
type: debuggerTypeName,
223+
name: 'Attach',
224+
port: 1234,
225+
subProcessId: 2,
226+
purpose: ['debug-test'],
227+
};
228+
229+
const session: any = {};
230+
getWorkspaceFoldersStub.returns(undefined);
231+
startDebuggingStub.resolves(true);
232+
233+
await attachService.attach(data, session);
234+
235+
// The original data object must not be mutated.
236+
expect(data).to.have.property('purpose').deep.equal(['debug-test']);
237+
});
192238
});

0 commit comments

Comments
 (0)