search

coreybutler / node-windows

Windows support for Node.JS scripts (daemons, eventlog, UAC, etc).
Other
2.8k stars 356 forks source link

Allowing Processes to shutdown cleanly #154

Closed hockeytim11 closed 7 years ago

hockeytim11 commented 7 years ago

I have a service that runs jobs. I wanted to be able to shutdown the service without any of the jobs failing.

Currently, the winsw executable sends a "Ctrl-C" signal to all processes in the process group. This causes everything to exit, including any child processes that may have been spawned. However, winsw has an XML configuration parameter called "stopparentprocessfirst". This change exposes that feature as a configuration option. It also exposes the "stoptimeout" property.

The second issue is that calling child.kill("SIGINT") causes the process to terminate on windows (see the note here).

To get around this issue, I send a child.send('shutdown') message instead. The child process needs to listen for that message and know to shut itself down. Finally, when the child process exits, we can exit the wrapper.

Just a note, I merged the changes from the arthurblake fork so I could get the .net4 fix as well as his changes for xml handling.

This tool has made it much easier to handle windows services. Thank you!

Also, I will be looking into node-linux next to make sure it can do the same thing.

coreybutler commented 7 years ago

First and foremost, thank you! These updates have been needed for quite some time, so I'm super-appreciative of your efforts.

My first glance didn't throw any red flags, but I'd like to run through this over the weekend to familiarize myself with everything.

hockeytim11 commented 7 years ago

I'm more interested in merging changes into this project rather than maintain my own fork. So let me know if there is something I can do to help make it easier to get some of these fixes in.

For instance, I was considering taking another stab at the .net4 fixes from scratch. It looks like the winsw project is on 2.0 now and has .net4 support built in. I'm not sure what all that entails at the moment, but if I head down that path, I would be happy to create a separate pull request with just that feature first. And then see about adding the clean shutdown as a follow on PR.

coreybutler commented 7 years ago

It typically is easier to merge in one feature at a time, but what you've done is something that has been on the to-do list for far too long. I'd actually like to merge everything... I really just want to make sure I'm familiar with the changes and keep an eye out for anything that may differ from the other node-* projects.

If you are interested in getting features into the project at a faster pace, I'd be open to bringing you in as another maintainer/collaborator. This PR would be a great first step for that.

coreybutler commented 7 years ago

I didn't get to check as much as I would have liked, but what I did looks good. I decided to merge this and released it as 0.1.13.