emacs-lsp / dap-mode

Emacs :heart: Debug Adapter Protocol
https://emacs-lsp.github.io/dap-mode
GNU General Public License v3.0
1.3k stars 182 forks source link

Multi-process debugging: dap-python and debugpyAttach/startDebugging #764

Open dsyzling opened 10 months ago

dsyzling commented 10 months ago

I've recently been hit by an issue which has caused dap-mode and dap-python to hang debugging a single process python app (via debugpy). This is caused by a change to pytables, or the underlying hdf5 library running on Ubuntu which starts a subprocess when opening an hdf5 file. When the subprocess starts debugpy sends a debugpyAttach request asking the debug client to attach to the subprocess. The dap-python module has an empty handler for debugpyAttach so the debugger hangs in Emacs.

The debugpyAttach request is proprietary but has since been replaced (well both are supported by debugpy) with an official startDebugging request which is part of the dap protocol. Specifically this has been added to support handling multi-process debugging. Searching some of the issues associated with dap-python here I'm sure some of them can be attributed to not handling debugypyAttach and/or multi process debugging.

I'm not a dap protocol or dap-mode expert but after investigation I have some working code but in order to resolve this and potentially support startDebugging it would impact dap-mode and not just dap-python. Currently I've forked dap-mode over on my github account - https://github.com/dsyzling/dap-mode, I'll push a branch later today with work in progress.

In order to resolve the issue however, I've implemented hierarchical debug sessions (parent/children) to support subprocesses. This was necessary to support creating a new sub process and retaining the parent process. When the subprocess is terminated we can switch back to the parent session - since dap-mode has a concept of 'current session'. The sub process is also added to the list of workspace sessions in order to be able to support breakpoints being set whilst debugging the parent or sub process. Secondly we may need to switch the current debugging session when a breakpoint is triggered.

In reading some of the issues and PRs over in the debugpy repos, a new session needs to be initiated when debugpyAttach or startDebugging is requested. This session needs to be initialised and the debug client needs to attach using the parameters passed within the original debugpyAttach/startDebugging message. For startDebugging I also found that responded to the startDebugging message at the end with success triggered to process to complete after the attach.

As an example of the kind of code I've explored here's a handler for debugpyAttach which relies on a some functions added to dap-mode - as well as changes to the debug session to maintain a parent debug session and child debug sessions.

(defun dap-python--handle-debugpy-attach (debug-session parsed-msg)
  "Handle PARSED-MSG request to debug subprocess from DEBUG-SESSION.

debugpyAttach will be sent as a request when a subprocess is spawned by the
parent debugger session. The dap-client should initiate a new session,
initialise the session and attach with parameters provided by debugpy
within the original messasge.

This function creates a child session of the parent DEBUG-SESSION
maintaining a hierarchy of debug sessions to manage multiple processes."
  (-let* (((&hash "name" name "connect" connect) parsed-msg)
          (launch-args (cl-copy-list (dap--debug-session-launch-args debug-session))))
    ;; modify launch args for our new create session
    (plist-put launch-args :request "attach")
    (plist-put launch-args :name (dap--calculate-unique-name name (dap--get-sessions)))
    (plist-put launch-args :dap-server-path nil)
    (plist-put launch-args :host (gethash "host" connect))
    (plist-put launch-args :debugServer (gethash "port" connect))

    ;; start new session for child process
    (let ((new-session (dap--create-child-session debug-session launch-args)))
      ;; send initialise
      (dap--send-message
       (dap--initialize-message "python-run-with-python-path")
       (dap--session-init-resp-handler
        new-session
        (lambda (_result)
          ;; send attach with arguments given to us by startDebugging/debugpyAttach message.
          (dap--send-message
           (dap--make-request "attach" parsed-msg)
           (lambda (_result))
           new-session)))
       new-session))))

Helper functions support parent/child debugger sessions:

(defun dap--create-child-session (parent-session launch-args)
  "Create a new child debugging session to debug a subprocess.

The new session will be created as a child session of PARENT-SESSION
using the LAUNCH-ARGS provided. 

The new child debugging session will be returned."
  (let ((new-session (dap--create-session launch-args))
        (debug-sessions (dap--get-sessions)))
    ;; add this session to our parent debug session
    (dap--add-child-debug-session parent-session new-session)
    ;; set parent of our new session
    (setf (dap--debug-session-parent-debug-session new-session) parent-session)
    ;; add to all workspace sessions so that breakpoints can be set correctly.
    (dap--set-sessions (cons new-session debug-sessions))

    new-session))

(defun dap--add-child-debug-session (parent-session child-session)
  "Add CHILD-SESSION as a child debugging session of PARENT-SESSION."
  (push child-session (dap--debug-session-child-debug-sessions parent-session)))

(defun dap--remove-child-debug-session (parent-session child-session)
  "Remove CHILD-SESSION from the parent debugging session PARENT-SESSION."
  (setf (dap--debug-session-child-debug-sessions parent-session)
        (delete child-session (dap--debug-session-child-debug-sessions parent-session))))

Changes were made to the 'stopped' event handler to switch the current session if a sub process breakpoint has been triggered. Modifications were made to 'terminated' to check for a parent debug session and switch back to the parent if the child was being terminated.

In my limited testing this resolves the issue of a pandas/pytables hanging when opening an hdf5 file. I've also tested this with a simple parent, child process, setting breakpoints, debugging the parent and child which seems to work well. I'm not confident that this will handle all scenarios due to my limited experience with dap-mode in general and the affect on other languages.

If I could localise this to dap-python I'd feel more comfortable, however given startDebugging is part of the standard I thought that it might be worthy of wider discussion in case other language models need a concept of multi-process debugging. Thankfully I didn't need to change dap-ui, although a tree of sessions may be more suitable to show parent/child relationships if it was thought this approach was suitable. In terms of VSCode this is the way they have implemented multi process debugging.

dsyzling commented 10 months ago

I noticed in the latest dap-mode code - testing and integrating some of my changes there is support for the startDebugging request. This isn't enabled within the initialisation parameters and also currently doesn't work with debugpy. The processing filter returns:

error in process filter: or: :debugServer or :dap-server-path should be present error in process filter: :debugServer or :dap-server-path should be present

I'm not sure what the current status of this work is? For this to work dap-server-path would have to be nil and debugServer should be set to the port from the original message. Looks like this function was added as part of 'Initial integration of new node js debugger' by @yyoncho .

dsyzling commented 10 months ago

This is an example startDebugging request message sent by debugpy in a multi-process scenario. In this case the launch-args are different from the scenario tested with the node js debugger. Here we may have launched a process with a straight command line (no attach), but the second process spawned is now requesting that we create a new session and attach to the sub-process.

{
  "seq": 22,
  "type": "request",
  "command": "startDebugging",
  "arguments": {
    "request": "attach",
    "configuration": {
      "type": "python-run-with-python-path",
      "name": "Subprocess 21490",
      "module": "package.runner",
      "cwd": "/home/dsyzling/dev/dap-multi-process",
      "python": [
        "/home/dsyzling/miniconda3/envs/dap-multi-process/bin/python"
      ],
      "isOutputRedirected": null,
      "subProcessId": 21490,
      "connect": {
        "host": "127.0.0.1",
        "port": 57819
      }
    }
  }
}
dsyzling commented 10 months ago

Something like this (excuse the poor emacs lisp and dash binding), which checks to see if there's a nested "connect" key and uses this to initiate the new session. However I think this still fails when the child session terminates and doesn't restore the current connection back to the parent?

(lsp-defun dap--start-debugging ((debug-session &as &dap-session 'launch-args 'proc 'name)
                                 (&hash "seq" "arguments"
                                        (&hash "configuration" (&hash "connect")
                                               "request" "configuration")))
  (-let* (((&plist :debugServer port :host) launch-args)
          (new-launch-args (list
                            :host (if connect (gethash "host" connect) host)
                            :debugServer (if connect (gethash "port" connect) port)
                            :request request
                            :name (format "Child of %s" name))))
    (ht-aeach (plist-put new-launch-args (intern (concat ":" key)) value) configuration)
    (dap-start-debugging-noexpand new-launch-args)

    (dap--send-message (dap--make-success-response
                        seq "startDebugging")
                       (dap--resp-handler) debug-session)))
Nosterx commented 7 months ago

Facing similar issue now. Did you managed to get it working?

dsyzling commented 7 months ago

I was hoping this would provoke a discussion on how to resolve the issue and possibly support hierarchical debugging. I made changes to dap-mode.el and dap-python in my forked repos above (https://github.com/dsyzling/dap-mode) and I'm currently running this locally using load-path (use-package) with my local changes. I haven't updated and merged any recent changes from dap-mode however so there may be some further work required. It's been working for me - but I haven't revisited it due to my current workloads - but I've been using it on a daily basis - albeit in my possibly limited scenarios with Python processes.

The last time I checked some work had been done to supporting startDebugging but it appeared to be for NodeJS. Debugpy had added support for hierarchical debugging and so the changes made for NodeJS didn't cope with subprocesses. My forked repos was an initial attempt to support parent/child relationships, however that change would not be limited to Python alone and would affect other language environments/debuggers. Hence why I was asking for a discussion on proposed ways we could accommodate hierarchical processes more widely.

Nosterx commented 7 months ago

Sadly this doesn't seems to be working for me. I installed your branch in doom emacs with

;; packages.el
(package! dap-mode
  :recipe(
    :host "github"
    :repo "dsyzling/dap-mode"
    :branch "python-multi-process"))

(package! dap-python
  :recipe(
    :host "github"
    :repo "dsyzling/dap-mode"
    :branch "python-multi-process"))

then ran

doom sync
doom build

Open my test and "Python :: Run pytest (at point)" and stil debugging hangs on

{
  "seq": 36,
  "type": "event",
  "event": "debugpyAttach",
  "body": {
    "type": "python",
    "module": "pytest",
    "name": "Subprocess 1293730",
    "python": [
      "/home/user/project/path/.venv/bin/python"
    ],
    "isOutputRedirected": null,
    "subProcessId": 1293730,
    "connect": {
      "host": "127.0.0.1",
      "port": 48325
    },
    "request": "attach"
  }
}

full log here debugpy_log.json

dsyzling commented 7 months ago

I might not be able to help with this, if it's related to some kind of issue with Doom Emacs - although I can't think why that would be an issue. Personally I'm running Emacs 29 under WSL Ubuntu. The fact that it hangs on startDebugging would lead me to believe you have a recent version of debugpy - I'm running 1.8.1.

Could you take pytest debug out of the picture and just run a simple script:

from multiprocessing import Process

def f(name):
    print('hello', name)

if __name__ == '__main__':
    p = Process(target=f, args=('bob',))
    p.start()
    p.join()
    print('Returned')

Test putting breakpoints on the two print statements and run this as a standard python script using M-x dap-debug.