Node: Debug already running process using v8-inspector?

[roblourens]

Currently you can send SIGUSR1 to a Node process to put it into debug mode as if it had been launched with --debug. Docs here. Is there any way to do the same using the new v8-inspector protocol?

[joshgav]

Perhaps we should add SIGUSR2 to activate inspector and leave SIGUSR1 to activate the old protocol?

Working impl of this here: https://github.com/joshgav/node/commit/0edde394cf3ef3ff815bf17b96cc223fc58e582d

/cc @nodejs/diagnostics

[ofrobots]

The problem is that there aren't enough signals, and SIGUSR2 might already be in use by user-space modules. Repurposing SIGUSR2 would be a semver major change.

I think a better approach would be to deprecate the old debugger in v8.x and free up SIGUSR1 for use by the inspector based protocol. Unfortunately this means that we won't have a solution to attach inspector to an already running process for v7.x and v6.x.

[papandreou]

SIGUSR2 is used by https://github.com/bnoordhuis/node-heapdump (triggers a heap dump).

[joshgav]

Is there a reasonable way to parameterize signals so we could double up SIGUSR1? One approach could be a config file next to the node binary.

/cc @bnoordhuis re node-heapdump. Do we know of other modules which use SIGUSR2?

@ofrobots:

we won't have a solution to attach inspector to an already running process for v7.x and v6.x

IMHO this would be a significant problem for inspector protocol adoption and Node diagnostics in general. We can't expect tools to use a different protocol for attaching to a running process, which is an important use case.

Waiting till v8.x (summer 2017) is too long.

deprecate the old debugger in v8.x and free up SIGUSR1

Even if/when we deprecate the old protocol I don't think we can change behavior of SIGUSR1 for another release or several. Tools will continue to support the old protocol, and v4.x will still be around till Oct 2017 at least.

[bnoordhuis]

The inspector using SIGUSR2 is not a problem for node-heapdump, it will simply override the inspector's signal handler. That may be a problem for the inspector, though. :-)

[ofrobots]

Is there a reasonable way to parameterize signals so we could double up SIGUSR1? One approach could be a config file next to the node binary.

This is not a bad idea. Another idea would be a flag, e.g. --inspect-on-sigusr1. /cc @nodejs/v8-inspector.

[roblourens]

I like the config file option, in that if there is a flag to set when you start node, people will inevitably forget to set that flag, then if you have to restart the process anyway you might as well start it in debug mode. I don't know whether there's some typical way to handle this kind of scenario though.

[joshgav]

Right, seems that if we could go back and restart with a flag we could just use --inspect itself. We need a way to notify and change a _running_ process, which suggests a signal. Any alternatives to signals?

Assuming we have to use a signal, it seems we'd prefer to reuse SIGUSR1. Even if we accepted the semver-major aspect of using SIGUSR2, I think taking the only other user signal would be inconsiderate to @bnoordhuis and others. (I didn't realize earlier that there are only 2! 😮 )

However, since SIGUSR1 is already in use in Node we need to parameterize it somehow so Node knows whether to start the old debugger agent or the new inspector agent. I suggested a config file and will throw together a patch to demonstrate. Maybe one day when the old debugger is gone we can remove it. Are there other patterns to somehow parameterize signals?

[bnoordhuis]

I don't like the idea of a config file. Node has always been a self-contained binary and it should stay that way.

[ofrobots]

Config files don't have a precedent, and quite a few issues will need to be worked through: does the config belong in the node installation, or does it belong with an application? Where do you find the config relative to the application? Should it be something in the package.json? What about standalone scripts, etc.

Other options:

• An environment variable.
• A named pipe that Node listens on.

[imyller]

+1 for --inspect-on-sigusr1 flag along with support for matching environment variable (e.g. NODE_INSPECT_ON_SIGUSR1).

Flag alone would be handy but sometimes you want to use exported environment variable to enable some behaviour in child processes too.

Flag wouldn't be semver-major change, right?

[bnoordhuis]

I'd say semver-minor, it's additive.

[joshgav]

@bnoordhuis @ofrobots Agreed, I also don't like the config file, I think it's too complex for the problem at hand.

But env vars or flags have to be set _before_ the process is run, and we need something to modify a _running_ process. Am I perhaps missing something?

@ofrobots - Named pipe is interesting, would that completely replace the signal?

Another idea - Perhaps we can add something to the existing debug agent which would allow us to ask it to switch to the inspector agent immediately? This would certainly be a more complex implementation, and there would be a slight perf penalty when a debug session starts, but that seems okay since it's intended for debug time and the tradeoff would be a better user experience in tools.

Also, once we're ready we can make the inspector agent the default and drop this (or more likely swap it to enable the old debugger agent).

[eugeneo]

Currently both inspector and debugger have their own server socket implementations. I could try to refactor that into a separate codebase that will start an appropriate agent based on whether the first massage is an HTTP request... I don't think it would be possible to have both agents running at the same time - so once the debugger or inspector is started, second implementation will never run.

[eugeneo]

Is there a roadmap for deprecating the debugger agent?

[joshgav]

@eugeneo

start an appropriate agent based on whether...

This sounds good to me! Once doing it though it might be worth giving it more flexibility than just switching on the protocol itself. Perhaps you could include a simple way to parameterize that first request, could be just a query param in the URL?

Is there a roadmap for deprecating the debugger agent?

IMHO we should wait to even discuss till v8.x timeframe (i.e. summer 2017) to see what state of ecosystem is - e.g. tools, helper modules, etc.

So for purposes of this thread I think we should consider the debugger agent the default and inspector agent secondary.

[ofrobots]

The old debug protocol in V8 has been deprecated for a number of years. Now that v8_inspector has just landed in V8 itself, I expect the old debug protocol is probably going to be removed late this year or early next year. I think we will need to have deal with the deprecation in the v8.x release.

[ofrobots]

Another idea: if the ports are different, perhaps we can start both debug servers when SIGUSR1 arrives (and optionally close the other port one when the first port gets a connection.)

[auchenberg]

I like that approach @ofrobots. How do we move forward from here? This functionality will enable a much better user experience for 3rd party devtools.

[bnoordhuis]

@auchenberg If you want to work on this, the logic is in DispatchDebugMessagesAsyncCallback and StartDebug/EnableDebug in src/node.cc.

[joshgav]

@bnoordhuis @auchenberg let's make sure we have consensus on what behavior should in fact be implemented. In the last Diagnostics WG meeting (https://github.com/nodejs/diagnostics/pull/74) we tentatively said we'd wait till the current debugger can be deprecated and then switch the USR1 signal to activate the inspector gateway instead. But it'll probably be a while till we can truly deprecate the old debugger, so an interim solution may be needed.

Following are the options suggested in this thread, in reverse order. I'll use the term "debugger" to refer to the legacy debugger agent, and "inspector" for the new one.

1. Start both inspector and debugger on SIGUSR1, shut the unused one down when messages are received on the other.
   
   
   
   • Works because they listen on different ports by default (5858, 9229).
   
   
2. Start a small intermediary gateway on SIGUSR1 which will activate the debugger or inspector depending on initial messages.
   
   
   
   • Could interfere with inspector protocol specification and HTTP calls to /json metadata.
   
   
3. Listen on a pre-specified named pipe instead of a signal.
   
   
   
   • Message sent through pipe could be more parameterized and explicit than a signal.
   
   
4. Configuration file.
   
   
   
   • Significant implications for Node in general which we might not want to deal with.
   
   
5. Environment variable which indicates what SIGUSR1 should do.
   
   
   
   • Can this be injected into a running process?
   
   
6. --inspect-on-sigusr1 runtime flag to switch SIGUSR1 behavior for process.
   
   
   
   • Requires restarting the process, in which case one could specify --inspect.
   
   
7. SIGUSR2 activates inspector, SIGUSR1 activates debugger.
   
   
   
   • Incompatible with modules which already use SIGUSR2 for their own purposes.
   
   
   • Unfairly takes all user signals for Node.
   
   

I vote for [1] as it matches the current behavior quite closely and provides a smooth exit path when the debugger is deprecated.

[bnoordhuis]

Yes, option 1 seems like the way forward and is in fact what I thought had been agreed upon previously.

[auchenberg]

Option 1 sounds like the most pragmatic one.

@bnoordhuis C++ isn't my thing, so don't wanna try to get some hacky code in.

[clarkbw]

We'd prefer option 1 as well, I think that will work well for us and others implementing connections to node. Want to get this on the radar for @jlongster and @jasonLaster as well.

[ofrobots]

Option 1 sounds the best to me as well. I think it should be fairly straight-forward to implement. I am attaching the help-wanted label if someone wants to contribute a PR.

[mpareja]

Is there an easy way to launch attach the inspector programmatically?

If I can attach to a running instance using the normal SIGUSR1 signal and then invoke an expression that attaches the V8 inspector, I could then attach.

[eugeneo]

Currently there's no API for starting the V8 inspector. Part of the reason is that Inspector should be started as early as possible to record events like the script parses and such. Extensive testing is needed to better understand what will or will not work when inspector is started late.

[joshgav]

We discussed this in the Diagnostics WG meeting at Node Interactive (notes here). Consensus was to not implement option 1 as decided in this thread, and instead to switch SIGUSR1 to activate Inspector instead of the old debugger now in master/current, to land in a release as a semver-major change in 8.0.

Reasons for not implementing an interim measure as proposed in this thread included:

1. It's more complex than we originally thought to start both and properly stop one (others can elaborate further).
2. The old debugger must be deprecated and removed soon anyway (see #9789) and we can switch already now in master to inspector, so we might as well take that straightforward course.

@nodejs/collaborators are there opposing views on this?

Next step is to open a PR to switch the current behavior. That should close this issue.

[benjamingr]

I'm +1 on the suggested course of action as well as #9789

[jkrems]

If we create a PR to change the behavior, should the same behavior also remove node debug <pid> support since it will no longer work unless the target pid was started with a different version of node?

[eugeneo]

I am assigning this to me as I have most scaffolding ready for implementing the new handler for the signal.

The new signal handler will be enabling the WebSocket server (i.e., start a worker thread and listen to server socket). The main issue is that V8 inspector instance will have to always be created during Node startup. This is required for it to install a number of hooks (e.g. console.log intercept).

[jkrems]

The main issue is that V8 inspector instance will have to always be created during Node startup.

Will this affect things like the (potentially upcoming) network instrumentation as well? Or would we wait for Network.enable for those parts?

[eugeneo]

@jkrems It would wait for the enable call.

[auchenberg]

Has this landed?

[joshgav]

@eugeneo

The main issue is that V8 inspector instance will have to always be created during Node startup.

At the surface this seems non-trivial. Do we have benchmarks showing diff between Node with Inspector activated and not? Are there other concerns to think about with having it always on?

console.log intercept

Would this be in place immediately, or simply ready to go when a signal is received?

[ofrobots]

The inspector being present is equivalent to being able to run const Debug = vm.runInDebugContext('Debug') today. I think it is important that no debug port is open unless the user explicitly asks for one, either via a command line option or by sending a signal (as today).

[joshgav]

@ofrobots

no debug port is open

And to be explicit, the separate uv loop and thread would not be created until/unless a signal is received, right?

[eugeneo]

@joshgav
IO thread is not created until the signal is received. Note that there's still a separate signal handler thread (same as current debugger) - I considered reusing it for running the IO loop, but for now decided it will complicate the code without much gain.

[ofrobots]

Wholehearted +1 on not reusing the signal thread as the debug IO thread.

[ORESoftware]

Hey all, I am curious if I can debug an already running node process with node --inspect, is there a way?

[jkrems]

@ORESoftware It depends on the version of node the running process uses. If it's node 8, then you should be able to kill -USR2 ${pidOfTheNodeProcess} to enable the --inspect behavior after the fact.

[ORESoftware]

@jkrems thanks, how does that actually work?

https://lists.freebsd.org/pipermail/freebsd-questions/2007-August/156889.html

I guess Node.js itself defines that signal as to allow --inspect to work after the fact. I remember learning about this a few years ago.

[jkrems]

@ORESoftware Yes, exactly. node registers a "signal handler" that is called whenever the operating system (e.g. via a user running kill) sends a specific signal to the process. The signal handler for SIGUSR1 is registered here: https://github.com/nodejs/node/blob/2f8ddb27340a5e7e914dc1160846d4822e6352a2/src/inspector_agent.cc#L130

It will enable the inspect protocol using the options provided at startup. Which means that you can't influence the port for example at the time you're sending the signal (since there's no additional data for signals).

[mizzao]

you should be able to kill -USR2 ${pidOfTheNodeProcess} to enable the --inspect behavior after the fact.

@jkrems I think you mean kill -USR1 <pid>. USR2 causes a heap dump.

[eugeneo]

More portable way would be to use process._debugProcess. E.g.:

~ $ node &
[1] 194811
~ $ node -e "process._debugProcess(194811)"

Upd: from the REPL, you may also call it on yourself - process._debugProcess(process.pid)

[jkrems]

@mizzao Thanks! Yes, I can never remember which is which. Really should've double-checked. :)

Good point re: portability w/ process._debugProcess. We should consider turning that into a non-underscored method so it doesn't look quite as dirty.

[mizzao]

What's the difference between

node -e "process.debugProcess(<pid>)"

and

node inspect -p <pid>

?

[eugeneo]

First starts a WebSocket server so you may attach Chrome DevTools or other frontend. Later starts a command line debugger (that uses same WS server)

[mizzao]

Oh, I see. So it's equivalent to kill -USR1 <pid>?

[eugeneo]

That's exactly what it does on *NIXes. It gets more complicated on Windows (kill is n/a on Windows at all)

[june07]

@jkrems Looking for a little guidance here...

Is there a way to influence (ie change) the inspect port when starting the inspect web socket after runtime? Maybe process._debugProcess(process.pid) would be a good place to start looking/make changes? I've dug around a bit and I'm not sure exactly where to begin in the codebase... C++ or Javascript. My C++ is weak so I'm sure I'm missing a lot in that regard. Some expert help/finger pointing would be great!

My problem currently is coming from the fact that you can only start the inspect process pragmatically on the default port 9229 and that's it. I'd like to be able to start debug web sockets on random ports, such that doing so would be possible on more than a single running Node program.

Another question I had was... is it possible to programmatically shut down the web socket externally just the same but opposite to the way it was started up (ie SIGUSR1)?

@ORESoftware Yes, exactly. node registers a "signal handler" that is called whenever the operating system (e.g. via a user running kill) sends a specific signal to the process. The signal handler for SIGUSR1 is registered here:

node/src/inspector_agent.cc

Line 130 in 2f8ddb2

RegisterSignalHandler(SIGUSR1, StartIoThreadWakeup);
It will enable the inspect protocol using the options provided at startup. Which means that you can't influence the port for example at the time you're sending the signal (since there's no additional data for signals).

[jkrems]

My problem currently is coming from the fact that you can only start the inspect process pragmatically on the default port 9229 and that's it.

The problem is a basic property of unix signals: They don't pass any data. So from the outside there's only "send USR1" which is all process._debugProcess does in the end. What you can do is start the processes with --inspect-port=<unique port>. That will make sure that once you send the signal, they will start listening on that custom port.

Another question I had was... is it possible to programmatically shut down the web socket externally just the same but opposite to the way it was started up (ie SIGUSR1)?

That might be possible but is tricky for two reasons:

1. It breaks when multiple clients are involved. E.g. an editor and a profiling tool both trying to attach to the same process. The 2nd client would accidentally stop the debug connection, not only preventing itself from working but also breaking the 1st client's connection.
2. If the debugger is active, the VM might be paused. Which, depending on how the signal is handled, might prevent the signal handling code from running. This is more a technical challenge and might be solvable.

The first one is what makes it most likely not practical to use the same signal for either. What might be possible though is to call process.stopDebug (forgot the exact function) via Runtime.evaluate.