Discussion:
option to wait for pid file to appear
(too old to reply)
Lennart Poettering
2018-05-17 17:23:15 UTC
Permalink
Hi,
I have a service unit for nginx that uses Type=forking and PIDFile.
That works, but occasionally I see in the log a message like
nginx.service: PID file /run/nginx/nginx.pid not readable (yet?) after
start: No such file or directory
So yes, this is a bug in nginx. They really should fix that. And this
is not only broken when you use systemd, but on sysvinit too, as a
command like this would likely fail there too: "service nginx start
service nginx status", as the start would return before the PID file
is written, and then status would claim the service to be down...

Hence, please file a bug against nginx asking them to wait in the
parent until the PID file is fully written before exiting. It's how
this works on SysV systems, and systemd wants that too.
but that busy waits. Is there any option to replace this hack via a
native systemd solution, like explicitly waiting for the pid file to
appear before considering the unit ready?
As others indicated, the best approach would be to do Type=notify, and
patch nginx to send out the right ready message after it wrote the PID
file. That'd be the native systemd solution.

PID files suck (as they have no clear clean-up regime), but if
services use them, then they should really implement them properly,
and thus write them before exiting in the forking parent.

Lennart
--
Lennart Poettering, Red Hat
Lennart Poettering
2018-05-17 17:25:46 UTC
Permalink
Post by Lennart Poettering
but that busy waits. Is there any option to replace this hack via a
native systemd solution, like explicitly waiting for the pid file to
appear before considering the unit ready?
this is a bug in nginx
however, after many years with systemd on Fedora one needs yet to show
me a service where you can't simply remove the PIDFile line and you are done
systemd is able to guess the main PID of a process in many cases, but
this cannot work in all cases, because from the outside we have no
clue what might be the main process and what is not. In particular
HTTP servers which fork off multiple worker processes are candidates
where systemd won't guess right, because there's ambiguity as there
are likely multiple processes around after start.

Hence, yes, ideally all services would use Type=simple or
Type=notify. But if they don't they should use Type=forking and that
only in combination with PIDFile=, otherwise service management
becomes black magic...

Lennart
--
Lennart Poettering, Red Hat
Igor Bukanov
2018-05-17 10:44:07 UTC
Permalink
It _is_ better for the PID file to be written out before the initial
process exits, but systemd will handle things correctly even if they
happen the other way around. Essentially the service won't be considered
to have completed activation until both events occur. If one or the other
takes too long (i.e. longer than the TimeoutStartSec= setting), the unit
will fail.
Does it means that even ExecStartPost and especially ExecReload can
always safely use $MAINPID? I.e. does systemd call these Execs only
after it managed to read the the pid file even when the file is
written after the initial ExecStart process exited?

And yes, Type=pid-file will be very useful. I have exactly a shell
script that will benefit from it nicely.
Michael Chapman
2018-05-17 11:03:28 UTC
Permalink
Post by Igor Bukanov
It _is_ better for the PID file to be written out before the initial
process exits, but systemd will handle things correctly even if they
happen the other way around. Essentially the service won't be considered
to have completed activation until both events occur. If one or the other
takes too long (i.e. longer than the TimeoutStartSec= setting), the unit
will fail.
Does it means that even ExecStartPost and especially ExecReload can
always safely use $MAINPID? I.e. does systemd call these Execs only
after it managed to read the the pid file even when the file is
written after the initial ExecStart process exited?
So it looks like systemd will add the MAINPID environment variable any
time it knows what the main PID is. It learns this by reading the PID
file, or if PIDFile= is omitted by guessing. It forgets it when it sees
the SIGCHLD from that PID and no new PID file is available.

So by the time ExecStartPost runs, the main PID may or may not be known.
Indeed, it's permitted for the PID file to be written out by ExecStartPost
itself.

For ExecReload, though, there definitely should be a known main PID. I
can't see any way you can get systemd to reload a service whose main PID
hasn't been determined (or guessed). ExecReload may cause systemd to
forget the main PID, if it makes the process exit, but it will be run with
$MAINPID set.
Post by Igor Bukanov
And yes, Type=pid-file will be very useful. I have exactly a shell
script that will benefit from it nicely.
I'll dig it out and see what I can do. It's probably bitrotted quite a bit
since I wrote it -- I never completed it because my own need for it went
away.
Lennart Poettering
2018-05-17 17:27:42 UTC
Permalink
Notice that there are 2 master processes, the old one with pid 2408
nginx can let PID 1 know about the new main PID by sending a MAINPID=
sd_notify() message.

Or even better, it could just push its fds into the fdstore and let
systemd take care of restarts without losing connections...

Lennart
--
Lennart Poettering, Red Hat
Reindl Harald
2018-05-17 10:29:39 UTC
Permalink
Have you tried without the PIDFile= setting at all?
As far as I can see that breaks live updates that nginx supports where
it starts a new process and workers and then gracefully terminates the
old main
this would be anyways broken because systemd reads the PIDFile once at
start to dtermine MAINPID and if MAINPID goes away the service fails
Igor Bukanov
2018-05-17 20:54:25 UTC
Permalink
Post by Lennart Poettering
So yes, this is a bug in nginx. They really should fix that. And this
is not only broken when you use systemd, but on sysvinit too, as a
command like this would likely fail there too: "service nginx start
service nginx status", as the start would return before the PID file
is written, and then status would claim the service to be down...
Given that systemd deals with this situation in a very reasonable way,
nginx must not be alone in doing this. And if this a common way to
write such code, perhaps it should not be considered a bug but rather
one more peculiarity of application services the service manager
should deal....
Lennart Poettering
2018-05-18 17:37:48 UTC
Permalink
Post by Igor Bukanov
Post by Lennart Poettering
So yes, this is a bug in nginx. They really should fix that. And this
is not only broken when you use systemd, but on sysvinit too, as a
command like this would likely fail there too: "service nginx start
service nginx status", as the start would return before the PID file
is written, and then status would claim the service to be down...
Given that systemd deals with this situation in a very reasonable way,
nginx must not be alone in doing this. And if this a common way to
write such code, perhaps it should not be considered a bug but rather
one more peculiarity of application services the service manager
should deal....
Well, no. The protocol is clear, and what we do is pretty close to
black magic, and still racy in many ways.

I mean, broken behaviour is still broken behaviour, even if we
nowadays handle it pretty nicely. Really, nginx should be fixed,
everybody benefits, sysvinit as much as we do.

Or, to turn this around: if nginx is happy that all only works
somewhat reliably on systemd systems, then they might as well just do
this properly, and rework this to Type=notify. Because right now,
systemd can deal with the broken behaviour, but *only* systemd really
can, and this will trip up on other systems, such as sysvinit or
upstart.

Lennart
--
Lennart Poettering, Red Hat
Lennart Poettering
2018-06-07 08:35:09 UTC
Permalink
Post by Lennart Poettering
Well, no. The protocol is clear, and what we do is pretty close to
black magic, and still racy in many ways.
I mean, broken behaviour is still broken behaviour, even if we
nowadays handle it pretty nicely. Really, nginx should be fixed,
everybody benefits, sysvinit as much as we do.
It turned out it is not only nginx that behaves this way. Any program
that is using the daemon(3) call available both in Linux and BSD
writes the pid from the child without the parent waiting. So this
behaviour must be widespread and as such I think must be accepted as a
de-facto standard (even if it is unfortunate) and systemd should not
even warn about it.
Nah, daemon() does not write PID files, you have to do that in your
own code. The fact that much code doesn't synchronize properly around
that is really a bug in that code. I mean, you can get away with using
daemon() as long as you synchronize around it, for example by first
allocating some pipe(), then waiting on it in the parent, and
signalling it from the child after having written the PID file...

Again, not synchronizing properly is broken for sysvinit as much as it
is broken for systemd. We deal with it to some point, but the only
fully safe and correct way to handle this is to fix the programs in
question.

Lennart
--
Lennart Poettering, Red Hat
Igor Bukanov
2018-06-07 09:07:27 UTC
Permalink
Post by Lennart Poettering
Nah, daemon() does not write PID files, you have to do that in your
own code.
As daemon() calls _exit() (not even exit()) in the parent after the
fork, the only way to synchronize the pid writing is not to use the
daemon() at all and inline daemon() functionality. I doubt that it
will ever be done for the wast majority of sources using the function.
And under systemd there are zero incentives to do that as the
workaround with waiting for the pid file to appear is good enough to
make things reliable. So as such the warning that currently systemd
produces is just a distraction that a user cannot do nothing about.

As a side note the usage of daemon() in 3 from 4 sources I randomly
picked up on github is wrong as the code calls the function and then
writes the pid file right after parsing the config before any
initialization of sockets, but that just pointed out how lack of
simple service initialization API lead to everybody inventing own
buggy code...
Michael Chapman
2018-06-07 08:27:30 UTC
Permalink
Post by Lennart Poettering
Well, no. The protocol is clear, and what we do is pretty close to
black magic, and still racy in many ways.
I mean, broken behaviour is still broken behaviour, even if we
nowadays handle it pretty nicely. Really, nginx should be fixed,
everybody benefits, sysvinit as much as we do.
It turned out it is not only nginx that behaves this way. Any program
that is using the daemon(3) call available both in Linux and BSD
writes the pid from the child without the parent waiting. So this
behaviour must be widespread and as such I think must be accepted as a
de-facto standard (even if it is unfortunate) and systemd should not
even warn about it.
I would agree. There's nothing the end user can really do about it, and
they're more likely to complain here about it than the problematic
software.

For what it's worth, I took a look at the Type=pid-file idea again... and
I'm not sure if I'm going to be able to do it. The problem is primarily
the way systemd only tracks up to one "control" process and one "main"
process, and that with Type=pid-file it's not clear at the outset whether
the process spawned by systemd is one or the other. I don't think starting
it off as a control process and then converting it to a main process -- if
and only if it writes its own PID out -- is the right approach.

So I might have to leave this idea for now, unless I can think of some way
around this. Sorry if you were waiting on it.
Reindl Harald
2018-05-17 21:14:09 UTC
Permalink
Post by Igor Bukanov
Post by Lennart Poettering
So yes, this is a bug in nginx. They really should fix that. And this
is not only broken when you use systemd, but on sysvinit too, as a
command like this would likely fail there too: "service nginx start
service nginx status", as the start would return before the PID file
is written, and then status would claim the service to be down...
Given that systemd deals with this situation in a very reasonable way,
nginx must not be alone in doing this. And if this a common way to
write such code, perhaps it should not be considered a bug but rather
one more peculiarity of application services the service manager
should deal....
why?

either fix the pidfile stuff in nginx or simply start supporting
"Type=notify" as a lot of other services already do

and NO i don't grasp why "live updates" need that at all instead have
main-pid as a tiny shim-controller which *never* exits at all
Ian Pilcher
2018-05-18 13:44:00 UTC
Permalink
Post by Igor Bukanov
Given that systemd deals with this situation in a very reasonable way,
nginx must not be alone in doing this. And if this a common way to
write such code, perhaps it should not be considered a bug but rather
one more peculiarity of application services the service manager
should deal....
IIRC, the Python daemon library does this.
--
========================================================================
Ian Pilcher ***@gmail.com
-------- "I grew up before Mark Zuckerberg invented friendship" --------
========================================================================
Zbigniew Jędrzejewski-Szmek
2018-05-17 14:07:09 UTC
Permalink
Have you tried without the PIDFile= setting at all?
As far as I can see that breaks live updates that nginx supports where
it starts a new process and workers and then gracefully terminates the
old main.
FTR, it *is* possible to do live updates like that. We added support
for that with FD_STORE in pid1, and systemd-journald and systemd-logind
now support restart without losing connections. It would be great if
nginx did that too.

Zbyszek
Jonathan de Boyne Pollard
2018-06-08 20:04:25 UTC
Permalink
Maybe socket-activation would work for you? (With Nginx it's also a
hack though.)
Accept=No
Environment=NGINX=3;

It is not terrifically complex. The documented way to stop the forking
is "daemon off", but an inherited listening socket also does it.

* https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=728015

*
https://github.com/nginx/nginx/blob/8e8734ec82cde91a02d0cbfaae0d0df6b5aaab14/src/core/nginx.c#L347
Continue reading on narkive:
Loading...