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

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

Open
hardfist opened this issue Apr 25, 2019 · 7 comments
Open

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

hardfist opened this issue Apr 25, 2019 · 7 comments
Assignees
Labels
v8 engine Issues and PRs related to the V8 dependency.

Comments

@hardfist
Copy link
Contributor

hardfist commented Apr 25, 2019

  • 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 bnoordhuis added question Issues that look for answers. wontfix Issues that will not be fixed. labels Apr 25, 2019
@bnoordhuis
Copy link
Member

bnoordhuis commented Apr 25, 2019

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.

@bnoordhuis bnoordhuis added duplicate Issues and PRs that are duplicates of other issues or PRs. and removed wontfix Issues that will not be fixed. labels Apr 25, 2019
@hardfist
Copy link
Contributor Author

hardfist commented Apr 25, 2019

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

@bnoordhuis
Copy link
Member

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
Copy link
Member

joyeecheung commented Apr 29, 2019

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
Copy link
Member

jasnell commented Jun 19, 2020

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

@joyeecheung
Copy link
Member

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

@joyeecheung joyeecheung added help wanted Issues that need assistance from volunteers or PRs that need help to proceed. and removed question Issues that look for answers. labels Oct 4, 2020
@joyeecheung
Copy link
Member

I am working on refactoring for #33807 and I think some of the work there might be useful for this. Tentatively assigning this to myself.

@joyeecheung joyeecheung removed the help wanted Issues that need assistance from volunteers or PRs that need help to proceed. label Oct 14, 2020
@joyeecheung joyeecheung self-assigned this Oct 14, 2020
@targos targos added v8 engine Issues and PRs related to the V8 dependency. and removed duplicate Issues and PRs that are duplicates of other issues or PRs. labels Dec 28, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
v8 engine Issues and PRs related to the V8 dependency.
Projects
None yet
Development

No branches or pull requests

5 participants