debugger doesnt stop when exiting extension development host

[chrisdias]

git clone https://github.com/microsoft/vscode-docker
cd vscode-docker && npm install
code-insiders .

F5 to launch the debugger
extension development host starts up
close the extension development host)

results:

• the debugger is still running, i have to press stop to actually stop the debugger
• pressing restart hangs with following message:

[weinand]

@bpasero I've debugged this on the debugger side and it seems that closing the EH window does neither terminate the EH node process nor does it send any broadcast events for this.

[bpasero]

@weinand this seems to be a regression from us moving to --inspect, I cannot reproduce the hanging extension host when I do not pass over the --inspect-brk to the extension host on launch.

Could it be that the debugger needs to explicitly send some signal to the debuggee to signal the session is over and otherwise the process will never terminate?

[bpasero]

/cc @alexandrudima because I know he refactored a lot of the extension host process recently to support restarting. I am not sure if there was any impact on how the process shutsdown (e.g. I see a newly introduced _cleanResources that seems to kill the extension host process on shutdown.

[weinand]

@bpasero yes, a terminate request from the debugger might be necessary.
@roblourens do you know something about this?

The problem is that the debugger doesn't know when the EH window's close button has been pressed. So we would at least need another broadcast event for this.

[weinand]

The issue is that process.exit() does not exit node if node has been started with --inspect.
Since I couldn't find any info about this behaviour I don't know whether this is a bug or feature.

@roblourens do you know anything about this? Could it be that there is a specific shutdown handshake between the runtime and the debugger is needed to exit the process properly?

Debugging this node program with the "inspector" protocol shows the same behaviour:

process.exit();
console.log("not reached");

The console.log statement is not reached but the node process doesn't exit.

Using process.kill(process.pid) in extensionHostMain.ts:29 instead of nativeExit(1) fixes the issue.

@alexandrudima @bpasero how should we proceed?

[bpasero]

If that works, why not use process.kill if the extension host knows that it is being debugged?

[weinand]

Yes, if Rob has no better ideas I suggest that we use process.kill(process.pid) if in debug mode.

[roblourens]

There was an issue in Node <8 where the process doesn't terminate or signal that it's complete until the debugger detaches. Now we get an event from node that the script has completed, and terminate the debugging session. This is probably related. I'm surprised that it impacts process.exit but I think process.kill is the right way to go.

[roblourens]

If it's ugly, we can revisit it with an electron version that has a node 8 equivalent (1.7.5 says node 7.9?) and maybe remove it then.

[weinand]

@alexandrudima could you please add the process.kill(process.pid) workaround the way you like it? Thanks.

[alexdima]

This is the nodejs PR that introduced this --- nodejs/node#7252

[weinand]

@roblourens this means it is a feature and our "inspector" DA should really detach when process.exit() is called.
But probably you can only detach in node versions >= 8.0 because only there you get an event for the process.exit(), correct?

[roblourens]

Correct. Original node2 issue: microsoft/vscode-node-debug2#11

It didn't occur to me that this would impact EH debugging, thanks for the investigation and fix. I opened #33196 to revisit this once Electron has Node 8.