Skip to content

Restore process creation functions in pydevd.stoptrace() - #2085

Open
Yashvardhan Thanvi (Thanatos9404) wants to merge 1 commit into
microsoft:mainfrom
Thanatos9404:fix-stoptrace-restore-process-functions
Open

Yashvardhan Thanvi (Thanatos9404) wants to merge 1 commit into
microsoft:mainfrom
Thanatos9404:fix-stoptrace-restore-process-functions

Conversation

@Thanatos9404

Copy link
Copy Markdown

Fixes #2014

With subProcess enabled (the default), settrace() replaces the functions that start new processes so that Python children connect to the debugger: _winapi.CreateProcess on Windows, os.fork, _posixsubprocess.fork_exec and subprocess._fork_exec on POSIX, and os.exec*, os.spawn* and os.posix_spawn* where they exist. pydevd.stoptrace() undoes the thread patches but not these. After it runs, every new Python child is still rewritten to call pydevd.settrace() against the adapter, which is gone by then, so the child exits with ConnectionRefusedError before running any of its own code.

On current main (Windows 11, CPython 3.13.2), after debugpy.listen() and pydevd.stoptrace(), subprocess.Popen([sys.executable, "-c", "print('child ok')"]) exits with code 1 and that traceback, with or without a client having been attached first. Resetting listen.called by hand, as in the issue, and calling debugpy.listen() again fails after 30 seconds with "timed out waiting for adapter to connect", because the adapter it launches goes through the same patched CreateProcess.

This adds undo_patch_new_process_functions() to pydev_monkey.py, and stoptrace() calls it right after undo_patch_thread_modules(). It covers both patch_new_process_functions() and patch_new_process_functions_with_warning(). For each function it puts the original back only while pydevd's wrapper is still the installed one, so a layer another library added on top later stays in place. gevent's patch_os() is one: it replaces os.fork and keeps the function it found, which is pydevd's wrapper. The original_* attributes are kept, because the wrapper looks the original up by that name and such saved references keep calling it. For this, monkey_patch_module() tags its wrappers with pydevd_original_name, and patches again when it finds the original back in place, so a later settrace() installs the wrappers again.

On the question in the issue: with this change stoptrace() alone is enough for Popen to work again, and the second debugpy.listen() above (with listen.called reset by hand) returns in under a second. Two things are unchanged. listen() still refuses a second call because of listen.called, and SetupHolder.setup keeps the first adapter's port, so Python children started during a second session still try the old adapter. Clearing SetupHolder.setup in stoptrace() fixed the latter in a local experiment. I left both out because they decide whether listening again is supported; I can add them here or in a follow-up if you want that.

Tests:

  • test_subprocess_after_stoptrace in tests/debugpy/test_multiproc.py runs with the four attach runners. The debuggee calls pydevd.stoptrace() after the client disconnects, starts python -c "print('child ok')" and reports the exit code and output. On main all four fail with assert [1, ''] == [0, 'child ok'] on Windows (3.13) and Linux (3.12); with this change all four pass.
  • In pydevd's tests_python/test_pydev_monkey.py, test_undo_patch_new_process_functions applies each patch function, undoes it and checks that the functions in os, subprocess, _posixsubprocess and _winapi are the ones from before. test_undo_monkey_patch_module uses a stand-in module: patch, undo, patch again, then wrap pydevd's wrapper in another function and check that the undo leaves that layer in place and that it still works. test_pydev_monkey.py: 33 passed on both platforms.
  • test_multiproc.py, test_disconnect.py, test_attach.py, test_api.py and server/test_api.py, run with pytest against src: the only failures (launch tests that time out in my environment, and attach to PID without the native binaries) fail the same way on main.
  • ruff check . passes.

settrace(patch_multiprocessing=True) replaces os.exec*, os.spawn*,
os.posix_spawn*, os.fork, _posixsubprocess.fork_exec,
subprocess._fork_exec and, on Windows, _winapi.CreateProcess, so that
child Python processes connect to the debugger. stoptrace() undid the
thread patches but left these in place. After it ran, every new Python
child was still rewritten to call pydevd.settrace() against an adapter
that no longer existed, and the child exited with ConnectionRefusedError
before running its own code. A later debugpy.listen() (with
listen.called reset by hand, as in the issue) timed out as well, since
the adapter it launches goes through the same patched function.

Add undo_patch_new_process_functions() and call it from stoptrace()
next to undo_patch_thread_modules(). It puts each original function
back only while pydevd's wrapper is still the installed one, so a layer
added on top later (gevent's os.fork, for example) stays in place. The
original_* attributes are kept, because such a layer, or any other saved
reference, still calls the wrapper, which looks the original up by that
name. monkey_patch_module() now tags its wrappers so that the undo can
recognise them, and patches again when it finds the original restored,
so a later settrace() installs the wrappers again.

Fixes microsoft#2014
@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
There may be pipelines that require an authorized user to comment /azp run to run.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Process creation after debugging session not possible: teardown needed

1 participant