Including this library may destroy other signal handlers
Closed this issue · 6 comments
index.js installs new SIGTERM and SIGINT handlers that may override your own custom work. Modules should not quietly install signal handlers in this fashion, especially ones that don't provide hooking and always exit with a zero (normal) status.
Hakuna matata! You're right! I'll look into that once I have a second. Feel free to send a pull request in the meantime.
Unfortunately it isn't really possible (from what I can tell) to achieve the same functionality. There can be only one handler for SIGINT and SIGTERM. They're Unix signals shoehorned by Node to be settable with a familiar EventEmitter syntax, but still remain fundamentally signals not events.
I think this is going to take a change that will break current functionality (and require changes to tests). I propose exposing 2 new functions: installHandlers and removePid. The first would install the SIGINT and SIGTERM handlers with reasonable defaults for those that don't need to do their own signal handling. The latter would be easily callable from one's own exit handlers to clean up pid files.
That seems fine. This will be a major release anyways.
The API will essentially be:
npid.install()
npid.create()
npid.remove()
2014-04-01 15:22 GMT-04:00 Kyle VanderBeek notifications@github.com:
Unfortunately it isn't really possible (from what I can tell) to achieve
the same functionality. There can be only one handler for SIGINT and
SIGTERM. They're Unix signals shoehorned by Node to be settable with a
familiar EventEmitter syntax, but still remain fundamentally signals not
events.I think this is going to take a change that will break current
functionality (and require changes to tests). I propose exposing 2 new
functions: installHandlers and removePid. The first would install the
SIGINT and SIGTERM handlers with reasonable defaults for those that don't
need to do their own signal handling. The latter would be easily callable
from one's own exit handlers to clean up pid files.Reply to this email directly or view it on GitHubhttps://github.com//issues/2#issuecomment-39247286
.
👍
Hey, if you want to double check that you're happy with this. I'll push to npm tonight or tomorrow.
Pushed 0.4 to npm: https://www.npmjs.org/package/npid