-
Notifications
You must be signed in to change notification settings - Fork 50
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
better to fail startup than auto-kill other running instances #324
Comments
Here is an excellent explanation on why it is better to leave daemon handling to external tools: https://stackoverflow.com/a/61681452 I.e. the recommendation there goes even further: Don't create a PID file at all - leave that to external tools as well! |
Thanks for pointing this out, @jonassmedegaard It's definitely true that external daemon management is superior to the ad-hoc solution I came up with. It's also not even used for the deployment script any more (I now use
So what should the behavior be? I think this would result in confusing error messages, like If I keep the PID file, I could have: Personally, I'd still rather have it would close the process, instead of having to then do this manually, but I'm pretty sure there are reasons why auto-closing is bad. So two questions:
|
Yes, handling background processes correctly is hard. Very hard. Might even be insecure - depending on how you fail to do it exactly correct. So don't do it yourself. Just please pretty please don't do it yourself. |
Ok, no real arguments still, but I know that I can trust your opinion on this. I'll remove the feature. |
Same downsides as killing someone sitting on your favorite seat in the bus: a) Might be a friend. b) Might be a stranger that is not an enemy.
Daemon handling is inherently platform-specific. The non-cross-platform alternative providing better UX is to use the platform-specific tools as recommended (same link as I referenced previously). If you feel like reinventing the wheel of systemd then here is a starting point: http://software.clapper.org/daemonize/ (but that doesn't solve cross-platform nor UX - systemd also provides APIs for improved UX). |
I stumbled upon #167 and that sems bad design to me: Normal behavior of daemons is to simply refuse to start if a pidfile exist and the referenced PID is alive.
It seems you wanted the forceful startup specifically for certain deployment scripts. I urge you to consider removing this functionality from atomic-server and instead handle such unusual forceful treatment of pre-existing processes e.g. by having your deployment script do a
killall atomic-server && sleep 2 || true
before starting atomic-server.Reason I stumbled upon #167 was that I wondered why
heim
was used. If you insist that forceful startup should be implemented in atomic-server itself, then at least consider moving to a more lightweight library for that - e.g.sysinfo
. I have zero experience programming in Rust, so only mention that alternative because others seem to recommend it, both at stackoverflow and users.rust-lang.org.The text was updated successfully, but these errors were encountered: