--cpu-prof not generate profile file when killed by SIGINT

[hardfist]

• Version: 12.0.0
• Platform: mac 10.12.4 && Debian 8
• Subsystem:

// app.js
// for more info about ReDoS, see:
// https://en.wikipedia.org/wiki/ReDoS

var r = /([a-z]+)+$/
var s = 'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa!'

console.log('Running regular expression... please wait')
for(let i=1;i<100;i++){
  console.time('benchmark');
  const s = 'a'.repeat(i) + '!'
  r.test(s);
  console.timeEnd('benchmark');
}

$ node --cpu-prof app.js 
$ kill -SIGINT {pid}

is this purpose ? or how can i get profile file when app killed by SIGINT?

[bnoordhuis]

It's intentional. There is only a very limited number of things you're allowed to do from a signal handler, see signal-safety(7). Creating and writing out a CPU profile isn't one of them.

edit: this is a partial duplicate of #24937 - specifically, it could be fixed if there was a watchdog thread for handling signals that communicates with the main thread.

[hardfist]

It seems that node-report can be generated by SIGUSR2, what makes cpu profile different? @joyeecheung

[bnoordhuis]

node-report uses a mechanism that integrates with the Node.js event loop, uv_signal_start(). For that to work however the event loop needs to make forward progress, something that won't happen when your script is stuck in a loop.

[joyeecheung]

I think we should be able to fix this based on best-effort - we can just not re-raise the signal directly but instead set something up, return normally, and then outside the handler, terminate the JS execution and do proper cleanups (like what we do for the REPL - which we also need to be careful about), including finishing the CPU profiling.

We already have a SIGINT watchdog implemented, it's only used in very few places but not globally, though I am not sure if it's adequate for this as we have workers that can profile their own threads but SIGINT is per-process so we may need extra synchronization. I'll take a look.

[jasnell]

Ping @joyeecheung ... are you still planning to look at this? Should this remain open?

[joyeecheung]

@jasnell Sorry for missing this...yes I think it should remain open, but i don't currently have time to look into it