sipcapture / heplify-server

HEP Capture Server for HOMER
https://sipcapture.org
GNU Affero General Public License v3.0
184 stars 85 forks source link

heplify-server.service has unnecessary ExecStop #435

Closed rlaager closed 4 years ago

rlaager commented 4 years ago

heplify-server.service has this:

ExecStop=/bin/kill ${MAINPID}

This is not necessary. The default behavior of systemd is to SIGTERM all the processes in the control group (followed by a SIGKILL if they don't die): https://www.freedesktop.org/software/systemd/man/systemd.kill.html#KillMode=

That is, you can just remove this line. The service stops fine for me if I remove that line (and systemctl daemon-reload to pick up the changes, of course).

(If for some reason the SIGTERM to all process in the control group is a problem, then the correct fix would be to set KillMode=mixed so the SIGTERM goes only to the main PID (as the ExecStop here is doing). I don't think that situation applies here, as heplify-server seems to be a single process anyway, so there are no other processes in the control group.)

Also, in general (though it's not actively harmful in this case), an ExecStop that just sends a signal is not good: "Note that it is usually not sufficient to specify a command for this setting that only asks the service to terminate (for example, by sending some form of termination signal to it), but does not wait for it to do so." -- https://freedesktop.org/software/systemd/man/systemd.service.html#ExecStop=

negbie commented 4 years ago

Every day I learn something new about systemd. Thanks for this. Would you like to join the contributors list or should I change it?

rlaager commented 4 years ago

You can change it. It's a one line deletion, and I don't really want to go through the overhead of submitting a PR. ;)

negbie commented 4 years ago

Lazy ppl like me do it via Github edit button ;)