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

#50 - cleanly exit postcss-server when signaled by client process #94

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

MaxMotovilov
Copy link

SIGTERM handler prevented process from ending - it has to stop the server explicitly in order for the rest of the cleanup logic to work as designed.

@coveralls
Copy link

coveralls commented Aug 30, 2019

Coverage Status

Coverage remained the same at 100.0% when pulling f7011a3 on MaxMotovilov:master into 55c18ce on wbyoung:master.

@wbyoung
Copy link
Owner

wbyoung commented Aug 30, 2019

Ok, this looks good to me. Any docs that point to this being a requirement or is this just anecdotal? Can you find any docs for this requirement?

Also, I'm not actually using this personally any more, so to merge with confidence, I'd love it if we could find someone else to verify that this works and doesn't break any use cases.

Finally, in order to merge, the CI build will need to pass.

Thanks for contributing!

@MaxMotovilov
Copy link
Author

Please review; see http://man7.org/linux/man-pages/man7/signal.7.html for the official documentation on Unix signals.

If you do accept the PR, could you publish the package to npmjs? As things are now, the published version is already behind the master branch.

@wbyoung
Copy link
Owner

wbyoung commented Aug 30, 2019

Right, I know how signals work. I was curious about needing to stop the server. I'm surprised that this wouldn't have been something that I ran into originally. Any other instances you know of that are similar to this?

@MaxMotovilov
Copy link
Author

MaxMotovilov commented Aug 30, 2019

Stopping the server executes the listener in your code that removes all signal handlers and then event queue drains and node exits. Calling process.exit() from the signal handler works too (that's what I tried first) but it executes the code removing socket file 2nd time around and throws an error. Calling server.close() was simply the path of least resistance.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants