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

Stopping/Restarting debugger fails to stop babel-node #57018

Closed
jairelton opened this issue Aug 22, 2018 · 8 comments

Comments

@jairelton
Copy link

commented Aug 22, 2018

  • VSCode Version: 1.26.1
  • OS Version: macOS 10.13.6

Steps to Reproduce:

  • Add babel-node to runtimeExecutable parameter in the lauch configuration.
  • Start the debugger
  • Stop the debugger
  • The node app continues to run in background.

If the app listen to a port, it will fail to start again with the error: EADDRINUSE

Lauch configuration:

{
    "type": "node",
    "request": "launch",
    "name": "Start",
    "program": "${workspaceFolder}/index.js",
    "runtimeExecutable": "${workspaceRoot}/node_modules/.bin/babel-node",
    "smartStep": true,
    "restart": false,
    "outputCapture": "std"
}

@isidorn isidorn assigned roblourens and weinand and unassigned isidorn Aug 22, 2018

@roblourens

This comment has been minimized.

Copy link
Member

commented Aug 22, 2018

Could you set "trace": true in your launch config, try it again, and upload the log here?

@jairelton

This comment has been minimized.

Copy link
Author

commented Aug 22, 2018

There you go: vscode_debug_log.zip

@roblourens

This comment has been minimized.

Copy link
Member

commented Aug 22, 2018

@weinand we aren't using terminateProcess.sh anymore so child processes aren't cleaned up.

I think that instead of this line https://github.com/Microsoft/vscode-node-debug2/blob/master/src/nodeDebugAdapter.ts#L518 we should invoke a version of terminateProcess.sh that sends SIGINT to all child processes.

@roblourens roblourens added bug debug and removed needs more info labels Aug 22, 2018

@roblourens roblourens added this to the August 2018 milestone Aug 22, 2018

@weinand

This comment has been minimized.

Copy link
Member

commented Aug 22, 2018

@roblourens In the July release we shipped this new feature:
https://code.visualstudio.com/updates/v1_26#_improved-stop-debug-behavior

By default a SIGINT sent to a process kills the process, which then takes down the child processes as well.
Sending SIGINT to the debuggee is the purpose of the terminate request (and supposed to be more graceful than killing it with the terminateProcess.sh script).

If the SIGINT is intercepted by the process (and does not result in a termination of the debuggee), the debug session continues and the red terminate button can be pressed another time.
This time the disconnect request is send to the DA which uses terminateProcess.sh as before.

This works as you can see with this snippet test.js:

const cluster = require('cluster');
const http = require('http');
const numCPUs = require('os').cpus().length / 2;

if (cluster.isMaster) {
	console.log(`Master ${process.pid} is running`);
	console.log(`Forking ${numCPUs} workers.`);
	// Fork workers.
	for (let i = 0; i < numCPUs; i++) {
		cluster.fork();
	}
	cluster.on('exit', (worker, code, signal) => {
		console.log(`worker ${worker.process.pid} died`);
	});
} else {
	// Workers can share any TCP connection
	// In this case it is an HTTP server
	http.createServer((req, res) => {
		res.writeHead(200);
		res.end('hello world\n');
	}).listen(8000);
	console.log(`Worker ${process.pid} started`);
}

run this with this launch config:

{
	"type": "node",
	"request": "launch",
	"name": "Cluster",
	"program": "${workspaceFolder}/test.js",
	"autoAttachChildProcesses": true
},

and then select the main process in the debug toolbar and terminate it.

Observe: all child processes terminate too.

@jairelton if you run your babel-node from the command line, does Control-C terminate it and its sub processes properly?

@jairelton

This comment has been minimized.

Copy link
Author

commented Aug 22, 2018

@weinand When I use babel-node from the command line, Control-C works as expected.

@roblourens

This comment has been minimized.

Copy link
Member

commented Aug 22, 2018

What I'm seeing is that ctrl+c works, but sending SIGINT doesn't. Also kill -SIGINT pid doesn't work. It seems like this is because ctrl+c sends the signal to the whole process group, which is not the same as what the code in node2 does.

I can send the signal to the process group like kill -SIGINT -pid, which does work. https://stackoverflow.com/questions/8398845/what-is-the-difference-between-ctrl-c-and-sigint

So in theory I can change the code in node2 to process.kill(-pid, 'SIGINT') although it isn't working right now but I'll investigate.

@roblourens

This comment has been minimized.

Copy link
Member

commented Aug 22, 2018

https://nodejs.org/api/child_process.html#child_process_shell_requirements

On non-Windows platforms, if options.detached is set to true, the child process will be made the leader of a new process group and session

It works if the process is started with the detached flag. So I think the fix is to set that flag probably only when the DA supportsTerminateRequest, and in terminate pass the negative pid to kill the group.

@roblourens

This comment has been minimized.

Copy link
Member

commented Aug 22, 2018

@jairelton if you want a workaround for now, you can set "console": "integratedTerminal"

roblourens added a commit that referenced this issue Aug 22, 2018

@octref octref added the verified label Aug 30, 2018

@vscodebot vscodebot bot locked and limited conversation to collaborators Oct 7, 2018

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
5 participants
You can’t perform that action at this time.