Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

dap.terminate doesn't kill all processes #446

Closed
David-Kunz opened this issue Feb 21, 2022 · 19 comments
Closed

dap.terminate doesn't kill all processes #446

David-Kunz opened this issue Feb 21, 2022 · 19 comments

Comments

@David-Kunz
Copy link
Contributor

David-Kunz commented Feb 21, 2022

Problem Statement

Hi,

In my plugin Jester I use the following code to terminate the current session

dap.terminate({}, {}, function()
  -- continue with further execution
end)

However, this does seem to terminate all processes or it won't terminate the next session which is about to be created afterwards.

What I usually do:

  • Debug a test
  • Debug a test again (in Jester, this calls dap.terminate({}, {}, cb) where cb debugs it again
  • continue until one has too many processes

Ideas or possible solutions

Is there a way to force nvim-dap to kill all running processes to have a clean state? Otherwise, over time, the amount of running processes makes my machine spin too much.

Thanks a lot!

@mfussenegger
Copy link
Owner

Wasn't this fixed in #343 ?

Do you know which process isn't terminated? the debug-adapter or the debugee?

Terminate shouldn't even be required, according to the specification if you're using a launch request the debugee should terminate by itself if it runs to completion.

Sounds a bit like a bug in the debug adapter. Unfortunately vscode-node-debug2 is no longer maintained :/
And the successor vscode-js-debug requires custom extensions. @entropitor tried to get it working once, but not sure if it worked out - see https://github.com/entropitor/dotfiles/commit/b39bf93d9e076b61e1214cff5ea039b35fcfe92b

@mfussenegger
Copy link
Owner

I'm also not sure what else nvim-dap could be doing. The launch request doesn't have any response - so nvim-dap doesn't know the pid of the process that gets started - it only knows the pid of a terminal if external or integrated terminal is used. The most I could do is expose that as part of the session so you could pick it up and attempt to kill it if it is still running.

@David-Kunz
Copy link
Contributor Author

David-Kunz commented Feb 22, 2022

Hi @mfussenegger ,

Thanks a lot for your response,

Wasn't this fixed in #343 ?

It seemed to work at the time of #343 , but now there are still some left-over processes. I recorded a video to demonstrate it.

The first few times I debug the test and wait until it finishes - all processes are destroyed in the end.
The last few times, I debug the test right after using terminate - there are some left-over processes.
processes.zip

The exact code to debug the test right after terminate is this:

local function terminate(cb)
  local dap = require('dap')
    print "calling terminate..."
    dap.terminate({}, {}, function()
      print "calling terminate finished..."
      cb()
    end)
 end

  terminate(function()
    return run(o) -- run calls `dap.run` in the end
  end)

I did another test: This time I stop at a breakpoint and call dap.terminate(), after a while, all node processes are correctly destroyed.
terminate.zip

Best regards,
David

@entropitor
Copy link
Contributor

For jest tests, I was able to get vscode-js-debug to work using this code indeed. (It wasn't working for normal debugging node applications for some reasons, those breakpoints were getting ignored. Setting breakpoints in VS Code and then attaching to the session did work, even for non jest stuff).

For jest it looks like it ignores the breakpoints as well but then eventually they do end up working. Potentially sending them to the right process (and not to others) might make it work for all things.

@David-Kunz
Copy link
Contributor Author

Thanks a lot for the information @entropitor and @mfussenegger !

Regarding Jest:
I also noticed ignored breakpoints, the reason for this is that there's some source code manipulation and the breakpoints are shifted a bit up/down. You can see it if you add a lot of breakpoints, eventually one will hit.

I noticed that it also works for node-debug2 but there is a source-maps problem. To make it work, you can set

  dap.run({
         -- ...,
        disableOptimisticBPs = true
      })

together with a .babelrc:

{
  "sourceMaps": true,
  "retainLines": true
}

@entropitor
Copy link
Contributor

@David-Kunz I think for the vscode-js-debug one, it might also have to do with the fact that they are being send to the "root session" which then rejects them and the actual "child" session doesn't reject them and so it works but I'm not sure, I never fully investigated.

@David-Kunz
Copy link
Contributor Author

I see, seems like a lot of trouble with vscode-js-debug, I think I'll stick to node-debug2 for now.

Regarding the processes: As a workaround, I will close the current process manually (require"dap".terminate()) before starting another debug session, then I don't have any problems.

The callback function doesn't seem to work in all cases and occasionally leaves some processes running:

    dap.terminate({}, {}, function()
      -- start next debug session here
    end)

@mfussenegger
Copy link
Owner

Could be that it doesn't set the terminateDebugee option if you call dap.terminate like that. Did you try with

    dap.terminate(nil, nil, function()
      -- start next debug session here
    end)

@David-Kunz
Copy link
Contributor Author

Will try immediately!

@David-Kunz
Copy link
Contributor Author

It seems to work (I restarted a debugging session ~10 times and don't have any more Node.js processes).

There is only a problem when I spam debugging processes (starting the next one without waiting until the one before is properly started), then I still have Node.js processes which are not terminated.

So I guess the terminate function is run before something else happens inside nvim-dap which then leads to nasty effects. Do you think there could be a way to detect when it's safe to call dap.terminate?

@mfussenegger
Copy link
Owner

Do you think you could create a minimal project and a minimal reproduction so I could look into it?

@David-Kunz
Copy link
Contributor Author

David-Kunz commented Mar 27, 2022

Hi @mfussenegger ,

Thanks for looking into it!

Minimal Example:

File: test.js

console.log('after')
debugger
console.log('after')

Lua configuration for node2:

dap.adapters.node2 = {
  type = 'executable',
  command = 'node',
  args = {os.getenv('HOME') .. '/apps/node/out/src/nodeDebug.js'}, -- or wherever your file is
}

Lua function to execute test:

_G.test_dap = function()
  local dap = require'dap'
  dap.terminate(nil, nil, function()
    dap.run({
      args = { "--no-cache" },
      console = "integratedTerminal",
      cwd = "<your_CWD>", -- <<<<<<<<<<<< please adjust!
      disableOptimisticBPs = true,
      port = 9229,
      protocol = "inspector",
      request = "launch",
      runtimeArgs = { "--inspect-brk", "test.js" },
      skipFiles = { "<node_internals>/**/*.js" },
      sourceMaps = "inline",
      type = "node2"
    })
  end)
end

Test:

  1. Call :lua test_dap() twice (each run stops at the debugger statement)
  2. Call :lua require"dap".terminate()
  3. Notice that there is one leftover node process running afterwards

Note:
If one would perform the following, there won't be leftover processes:

  1. Call :lua test_dap() once
  2. Call :lua require"dap".terminate() (and wait a second)
  3. Call :lua test_dap() again
  4. Call :lua require"dap".terminate() again

Best regards,
David

@mfussenegger
Copy link
Owner

I think the problem is that many parts of run are async. The session gets assigned before it is fully initialized. So the second terminate doesn't work because the just started session isn't ready yet

You could try adding something like a

  vim.wait(2000, function()
    local session = dap.session()
    return session and session.initialized
  end)

After the terminate, but not sure if that will help.

I also think that the debug adapter has a problem - because the logs say that the process gets closed and a debug-adapter launched in launch mode is supposed to cleanup the debug process if it exits.

@David-Kunz
Copy link
Contributor Author

If I perform

_G.test_dap = function()
  local dap = require'dap'
  dap.terminate(nil, nil, function()
    vim.wait(2000, function()
        dap.run({
          args = { "--no-cache" },
          console = "integratedTerminal",
          cwd = "your-cwd", -- <<<<<<<<<<<< please adjust!
          disableOptimisticBPs = true,
          port = 9229,
          protocol = "inspector",
          request = "launch",
          runtimeArgs = { "--inspect-brk", "test.js" },
          skipFiles = { "<node_internals>/**/*.js" },
          sourceMaps = "inline",
          type = "node2"
        })
    end)
  end)
end

Then I get the error message

Error executing vim.schedule lua callback: Vim:E903: Process failed to start: too many open files: "/usr/local/bin/node"

@mfussenegger
Copy link
Owner

Not surprising. Take a look at :h vim.wait :)

@David-Kunz
Copy link
Contributor Author

Oh I see, sorry for not reading the documentation. It waits until the callback is done - I thought it would wait and then call the callback... let me check again.

@David-Kunz
Copy link
Contributor Author

Okay, with the following

_G.test_dap = function()
  local dap = require'dap'
  dap.terminate(nil, nil, function()
    vim.wait(2000, function()
      local session = dap.session()
      return session and session.initialized
    end)
    dap.run({
      args = { "--no-cache" },
      console = "integratedTerminal",
      cwd = "...", -- <<<<<<<<<<<< please adjust!
      disableOptimisticBPs = true,
      port = 9229,
      protocol = "inspector",
      request = "launch",
      runtimeArgs = { "--inspect-brk", "test.js" },
      skipFiles = { "<node_internals>/**/*.js" },
      sourceMaps = "inline",
      type = "node2"
        })
    end)
end

it can restart, but previous sessions remain. The effect is that when stopped at a breakpoint, I can't continue. I get the error message, that there's no configuration.

@mfussenegger
Copy link
Owner

Short update:

There was a race where the callback passed to terminate wasn't always triggered. This should be solved with 03807a4 and ef44e47

That the JS adapter process sometimes refuses to die is a far as I can tell an issue of the adapter

@zyriab
Copy link

zyriab commented Sep 17, 2023

Hello, I'm working on integrating the user's npm scripts into the DAP config automatically, it works really well but the process does not get killed when the debug session is over.
It seems to be related to the adapter (nvim-dap-vscode-js) itself and there is an issue that's been open for a few months without any response from the maintainer, @mxsdev

I found out that if you use the integrated console when running the debugger with the option console = "integratedTerminal", focus said terminal, enter insert mode and press <C-c>, it does kill the process, so I thought going that route was an easier feat than trying to debug the adapter and opening a PR that will probably never be reviewed, since the adapter does not seem to be actively maintained anymore.

I read above that @mfussenegger talked about giving the console pid as part of the execution context, was this implemented? I'd love to be able to use this and append a function to the terminated/exited events in order to cleanly stop the running process. It would also be a better option in order to make my plugin adapter-agnostic.

Here's the config I'm running to launch npm scripts:

{
    name = "Run script",
    type = "pwa-node",
    request = "launch",
    cwd = "${workspaceFolder}",
    rootPath = "${workspaceFolder}",
    sourceMaps = true,
    skilpFiles = { "<node_internals>/**" },
    protocol = "inspector",
    console = "integratedTerminal",
    runtimeExecutable = "npm",
    runtimeArgs = {
        "run-script",
        "some_npm_script"
    }
}

Cheers

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

No branches or pull requests

4 participants