You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
SIGINT handlers defined in app are not executed #192
consttmp=require('tmp');console.log(`Run from terminal: kill -SIGINT ${process.pid}`);process.on('SIGINT',()=>{console.log('Custom SIGINT handler - do some clean up')process.exit(0);});setTimeout(()=>{},100000)
Custom SIGINT handler - do some clean up not printed.
The same code works OK with tmp@0.0.33
Reacted by Gleb Alexeyev, John Knox, Dreamer53, Eugene, Tyler Murphy, Samuel Ferencik and Dmitriy Pushkov
@alexandervain Very good find. This always went unnoticed in the tests as we do not have any custom SIGINT handlers installed. I wonder why the routine even checks the doExit parameter...
I think that this can be removed completely.
Ok, the doExit was there for a reason, having to do with jest and how it is managing its sandboxes, which eventually will run the initialising code of tmp multiple times.
I think I have nailed it down and will prepare a PR now.
There is also the case of async handlers - we have process.on() handlers that return promise, and NodeJS will wait for those promises to resolve before existing. As I understand df8df51, it will call original handlers in a sync matter, which will not give async functions enough time to complete.
Activity
silkentrance commentedon Apr 21, 2019
@alexandervain Very good find. This always went unnoticed in the tests as we do not have any custom SIGINT handlers installed. I wonder why the routine even checks the doExit parameter...
I think that this can be removed completely.
silkentrance commentedon Apr 26, 2019
Ok, the doExit was there for a reason, having to do with jest and how it is managing its sandboxes, which eventually will run the initialising code of tmp multiple times.
I think I have nailed it down and will prepare a PR now.
fix #192: tmp must not exit the process on its own
fix #192: tmp must not exit the process on its own
use tmp instead of temp
alubbe commentedon Jun 3, 2019
@alexandervain @papandreou Does this commit fix the bug? df8df51
fix #192: tmp must not exit the process on its own
Fix empty headless output bug
eugene1g commentedon Sep 8, 2019
There is also the case of async handlers - we have process.on() handlers that return promise, and NodeJS will wait for those promises to resolve before existing. As I understand df8df51, it will call original handlers in a sync matter, which will not give async functions enough time to complete.
Tyler-Murphy commentedon Sep 25, 2019
@alubbe, I encountered the same bug while doing asynchronous operations in a
SIGINTevent listener. It was fixed by df8df51.alubbe commentedon Sep 25, 2019
sounds good - how about merging #193 ?
Merge pull request #193 from raszi/gh-192