[systemd-devel] How to run forking daemon with systemd in correct way, without errors?

[Gena Makhomed]

Hello, All!

I am use nginx web server (http://nginx.org/) with systemd.
But I see errors in /var/log/messages when running nginx:

systemd: Starting nginx - high performance web server...
systemd: Failed to read PID from file /var/run/nginx.pid: Invalid argument
systemd: Started nginx - high performance web server.

systemd: Starting nginx - high performance web server...
systemd: PID file /var/run/nginx.pid not readable (yet?) after start.
systemd: Started nginx - high performance web server.

As I can see from systemd sources - these errors come from function
service_load_pid_file():
https://github.com/systemd/systemd/blob/master/src/core/service.c#L815

As I understand from comments in function service_enter_start():
https://github.com/systemd/systemd/blob/master/src/core/service.c#L1909

/* For forking services we wait until the start
* process exited. */

And comments in function service_sigchld_event():
https://github.com/systemd/systemd/blob/master/src/core/service.c#L2959

/* Forking services may occasionally move to a new PID.
* As long as they update the PID file before exiting the old
* PID, they're fine. */

Systemd think what service is fully started after start process exited?

But nginx is forking daemon - when start process exited -
no warranty what pidfile already created by child process.

This is bug in nginx code? And this bug should be fixed in nginx?

But "daemon(); write_pidfile();" is common pattern
used by many services and even in library functions.

I can ask nginx developers to fix bug, but don't see bugs in nginx code.

Or this is bug in systemd? And may be systemd doing something wrong?

I can't understand the root cause of these two error messages.

Can you please help me,

How to run forking daemon with systemd in correct way, without errors?

Unit file of nginx is simple:

/usr/lib/systemd/system/nginx.service

=====================================================================

[Unit]
Description=nginx - high performance web server
Documentation=http://nginx.org/en/docs/
After=network-online.target remote-fs.target nss-lookup.target
Wants=network-online.target

[Service]
Type=forking
PIDFile=/var/run/nginx.pid
ExecStart=/usr/sbin/nginx -c /etc/nginx/nginx.conf
ExecReload=/bin/kill -s HUP $MAINPID
ExecStop=/bin/kill -s TERM $MAINPID

[Install]
WantedBy=multi-user.target

=====================================================================

[Reindl Harald]

Post by Gena Makhomed
I am use nginx web server (http://nginx.org/) with systemd.
systemd: Starting nginx - high performance web server...
systemd: Failed to read PID from file /var/run/nginx.pid: Invalid argument
systemd: Started nginx - high performance web server.
systemd: Starting nginx - high performance web server...
systemd: PID file /var/run/nginx.pid not readable (yet?) after start.
systemd: Started nginx - high performance web server

either the daemon needs to be fixed or just remove the pid-file stuff
from the systemd-unit, systemd don't need it really

[Gena Makhomed]

Post by Reindl Harald

Post by Gena Makhomed
I am use nginx web server (http://nginx.org/) with systemd.
systemd: Starting nginx - high performance web server...
systemd: Failed to read PID from file /var/run/nginx.pid: Invalid argument
systemd: Started nginx - high performance web server.
systemd: Starting nginx - high performance web server...
systemd: PID file /var/run/nginx.pid not readable (yet?) after start.
systemd: Started nginx - high performance web server

either the daemon needs to be fixed

Ok, but can you tell, what is wrong with daemon?

this is nginx daemonization code:
http://hg.nginx.org/nginx/file/tip/src/os/unix/ngx_daemon.c

this is function for writing pidfile:
http://hg.nginx.org/nginx/file/tip/src/core/ngx_cycle.c#l944

Post by Reindl Harald
or just remove the pid-file stuff
from the systemd-unit, systemd don't need it really

pid-file need for service reload and service stop.

[Reindl Harald]

Post by Gena Makhomed

or just remove the pid-file stuff from the systemd-unit, systemd don't
need it really

pid-file need for service reload and service stop

nonsense - if maintain a ton of production servers from web/proxy over
mail and what not else and none hase a PID file config in the systemd-unit

please read how systemd works

[***@srv-rhsoft:/etc/systemd/system]$ cat
/etc/systemd/system/smb.service | grep -i type
Type=forking
[***@srv-rhsoft:/etc/systemd/system]$

[***@srv-rhsoft:/etc/systemd/system]$ cat
/etc/systemd/system/smb.service | grep -i pid
ExecReload=/usr/bin/kill -HUP $MAINPID
[***@srv-rhsoft:/etc/systemd/system]$

[***@srv-rhsoft:/etc/systemd/system]$ systemctl status smb
? smb.service - Samba SMB Daemon
Loaded: loaded (/etc/systemd/system/smb.service; enabled; vendor
preset: disabled)
Active: active (running) since Wed 2017-11-22 16:35:55 CET; 6h ago
Process: 977 ExecStart=/usr/sbin/smbd -D (code=exited, status=0/SUCCESS)
Main PID: 1197 (smbd)
Tasks: 4 (limit: 512)
CGroup: /system.slice/smb.service
??1197 /usr/sbin/smbd -D
??1198 /usr/sbin/smbd -D
??1199 /usr/sbin/smbd -D
??1257 /usr/sbin/smbd -D

[***@srv-rhsoft:/etc/systemd/system]$ systemctl reload smb
[***@srv-rhsoft:/etc/systemd/system]$ systemctl status smb
? smb.service - Samba SMB Daemon
Loaded: loaded (/etc/systemd/system/smb.service; enabled; vendor
preset: disabled)
Active: active (running) since Wed 2017-11-22 16:35:55 CET; 6h ago
Process: 30751 ExecReload=/usr/bin/kill -HUP $MAINPID (code=exited,
status=0/SUCCESS)
Process: 977 ExecStart=/usr/sbin/smbd -D (code=exited, status=0/SUCCESS)
Main PID: 1197 (smbd)
Tasks: 4 (limit: 512)
CGroup: /system.slice/smb.service
??1197 /usr/sbin/smbd -D
??1198 /usr/sbin/smbd -D
??1199 /usr/sbin/smbd -D
??1257 /usr/sbin/smbd -D

Nov 22 16:35:53 srv-rhsoft.rhsoft.net systemd[1]: Starting Samba SMB
Daemon...
Nov 22 16:35:55 srv-rhsoft.rhsoft.net systemd[1]: Started Samba SMB Daemon.
Nov 22 16:35:55 srv-rhsoft.rhsoft.net smbd[1197]: [2017/11/22
16:35:55.447806, 0] ../lib/util/become_daemon.c:124(daemon_ready)
Nov 22 16:35:55 srv-rhsoft.rhsoft.net smbd[1197]: STATUS=daemon 'smbd'
finished starting up and ready to serve connections
Nov 22 23:29:18 srv-rhsoft.rhsoft.net systemd[1]: Reloading Samba SMB
Daemon.
Nov 22 23:29:18 srv-rhsoft.rhsoft.net systemd[1]: Reloaded Samba SMB Daemon.

[Gena Makhomed]

Post by Reindl Harald

Post by Gena Makhomed

or just remove the pid-file stuff from the systemd-unit, systemd
don't need it really

pid-file need for service reload and service stop

nonsense

You are talking about GuessMainPID= option?

https://www.freedesktop.org/software/systemd/man/systemd.service.html#GuessMainPID=

You can read from documentation of this directive:
"The guessing algorithm might come to incorrect conclusions if a daemon
consists of more than one process."

nginx daemon consists of more than one process,
so directive MainPID= should be defined in unit file.

BTW, nginx has ability to change executable on the fly:
http://nginx.org/en/docs/control.html#upgrade

In any case, you are talking about something like workaround of the bug,
but I am asking in systemd-devel mail list about root cause of this bug,
how to fix this bug, and where bug is located - in nginx or in systemd?

Post by Reindl Harald
please read how systemd works

I read even sources of systemd and nginx.

But I still can't find *root cause* of this bug:

systemd: Stopping nginx - high performance web server...
systemd: Starting nginx - high performance web server...
systemd: PID file /var/run/nginx.pid not readable (yet?) after start.
systemd: Started nginx - high performance web server.

[Reindl Harald]

Post by Gena Makhomed

Post by Reindl Harald

Post by Gena Makhomed

or just remove the pid-file stuff from the systemd-unit, systemd
don't need it really

pid-file need for service reload and service stop

nonsense

You are talking about GuessMainPID= option?
https://www.freedesktop.org/software/systemd/man/systemd.service.html#GuessMainPID=

no i talk about nearly 6 years systemd expierience in production and
that i yet need to see a real world case where systemd don't work in
case of missing PID file

Post by Gena Makhomed
"The guessing algorithm might come to incorrect conclusions if a daemon
consists of more than one process."
nginx daemon consists of more than one process,
so directive MainPID= should be defined in unit file.

i know that all, httpd and postfix are typically forking services and
are working fine without any PID file since 2011

Post by Gena Makhomed
http://nginx.org/en/docs/control.html#upgrade
In any case, you are talking about something like workaround of the bug,
but I am asking in systemd-devel mail list about root cause of this bug,
how to fix this bug, and where bug is located - in nginx or in systemd?

in nginx which obviously creates the pid file too late

Post by Gena Makhomed

Post by Reindl Harald
please read how systemd works

I read even sources of systemd and nginx.

why do you then pretend the pid-file is needed for *service stop* when
systemd monitors every process of the cgroup and other then sysvinit can
guarantee that *every* single process which is still alive after send
SIGTERM to the MAINPID and don't exit get it's own SIGTERM and finally
SIGKILL to leftover processes no matter how they get started (shell_exec
from a php srcipt as example) - the CGROUP stuff and how systemd stops
services is basic knowledge

Post by Gena Makhomed
systemd: Stopping nginx - high performance web server...
systemd: Starting nginx - high performance web server...
systemd: PID file /var/run/nginx.pid not readable (yet?) after start.
systemd: Started nginx - high performance web server

likely the pid-file don't exist at the moment of forking

[Gena Makhomed]

Post by Reindl Harald

Post by Gena Makhomed

or just remove the pid-file stuff from the systemd-unit,
systemd don't need it really

You are talking about GuessMainPID= option?

no i talk about nearly 6 years systemd expierience in production and
that i yet need to see a real world case where systemd don't work in
case of missing PID file

You are argue not with me, you are argue with systemd documentation:

"...it is recommended to also use the PIDFile= option, so that systemd
can identify the main process of the daemon" -
https://www.freedesktop.org/software/systemd/man/systemd.service.html#Type=

Post by Reindl Harald

Post by Gena Makhomed
"The guessing algorithm might come to incorrect conclusions if a
daemon consists of more than one process."
nginx daemon consists of more than one process,
so directive MainPID= should be defined in unit file.

i know that all, httpd and postfix are typically forking services and
are working fine without any PID file since 2011

# grep PIDFile /usr/lib/systemd/system/postfix.service
PIDFile=/var/spool/postfix/pid/master.pid

httpd is not Type=forking service, it is Type=notify one.

# grep Type /usr/lib/systemd/system/httpd.service
Type=notify

Post by Reindl Harald

Post by Gena Makhomed
In any case, you are talking about something like workaround of the bug,
but I am asking in systemd-devel mail list about root cause of this bug,
how to fix this bug, and where bug is located - in nginx or in systemd?

in nginx which obviously creates the pid file too late

Creating pid file from grandchild process is common UNIX pattern.

Thousands of daemons are working in this way. Rewrite them all?

May be fixing systemd is far more simple than fixing all daemons?

[Reindl Harald]

Post by Gena Makhomed

Post by Reindl Harald
i know that all, httpd and postfix are typically forking services and
are working fine without any PID file since 2011

# grep PIDFile /usr/lib/systemd/system/postfix.service
PIDFile=/var/spool/postfix/pid/master.pid
httpd is not Type=forking service, it is Type=notify one.
# grep Type /usr/lib/systemd/system/httpd.service
Type=notify

please don't tell me what my services are, it can be Type=notify only
when mod_systemd is compiled in but there is not compelling reason to do so

[***@rh:~]$ rpm -q httpd
httpd-2.4.29-2.0.fc26.20171030.rh.sandybridge.x86_64

Post by Gena Makhomed

Post by Reindl Harald

Post by Gena Makhomed
In any case, you are talking about something like workaround of the bug,
but I am asking in systemd-devel mail list about root cause of this bug,
how to fix this bug, and where bug is located - in nginx or in systemd?

in nginx which obviously creates the pid file too late

Creating pid file from grandchild process is common UNIX pattern.
Thousands of daemons are working in this way. Rewrite them all?
May be fixing systemd is far more simple than fixing all daemons?

obviously the PID file don't exist when it is expected

http://thelinuxjedi.blogspot.co.at/2014/02/why-use-double-fork-to-daemonize.html
https://www.freedesktop.org/software/systemd/man/systemd.service.html

If set to forking, it is expected that the process configured with
ExecStart= will call fork() as part of its start-up. The parent process
is expected to exit when start-up is complete and all communication
channels are set up. The child continues to run as the main daemon
process. This is the behavior of traditional UNIX daemons. If this
setting is used, it is recommended to also use the PIDFile= option, so
that systemd can identify the main process of the daemon. systemd will
proceed with starting follow-up units as soon as the parent process exits.

[Lennart Poettering]

Post by Reindl Harald

Post by Gena Makhomed

or just remove the pid-file stuff from the systemd-unit,
systemd don't need it really

You are talking about GuessMainPID= option?

no i talk about nearly 6 years systemd expierience in production and
that i yet need to see a real world case where systemd don't work in
case of missing PID file

"...it is recommended to also use the PIDFile= option, so that systemd can
identify the main process of the daemon" -
https://www.freedesktop.org/software/systemd/man/systemd.service.html#Type=

If you use Type=forking, then you'll get away with not specifiying a
PID file in most cases, but it's racy as soon as you have more than
one daemon process, and nginx appears to be one of this kind, hence
please specify PIDFile=.

Creating pid file from grandchild process is common UNIX pattern.
Thousands of daemons are working in this way. Rewrite them all?

Well, the better written ones synchronize properly.

Lennart

[Clemens Gruber]

Hi,

Post by Gena Makhomed

Post by Reindl Harald

Post by Gena Makhomed
I am use nginx web server (http://nginx.org/) with systemd.
systemd: Starting nginx - high performance web server...
systemd: Failed to read PID from file /var/run/nginx.pid: Invalid argument
systemd: Started nginx - high performance web server.
systemd: Starting nginx - high performance web server...
systemd: PID file /var/run/nginx.pid not readable (yet?) after start.
systemd: Started nginx - high performance web server

either the daemon needs to be fixed

Ok, but can you tell, what is wrong with daemon?

There is nothing wrong with nginx per se. I am using a nginx.service
file similar to the following one:
https://git.pengutronix.de/cgit/ptxdist/plain/projectroot/usr/lib/systemd/system/nginx.service

If nginx was not built with --pid-path=/var/run/nginx.pid you have to
override it in your nginx.conf with the correct pid path, e.g.
pid /var/run/nginx.pid;
See http://nginx.org/en/docs/ngx_core_module.html#pid
(Your error looks like nginx did create the pid file somewhere else and
systemd can't find it)

Buildroot has a different approach. They add that pid config line, etc.
to the nginx command, so you do not have to add it to your nginx.conf:
https://github.com/buildroot/buildroot/blob/master/package/nginx/nginx.service

Cheers,
Clemens

[Gena Makhomed]

Post by Clemens Gruber

Post by Gena Makhomed

Post by Reindl Harald

Post by Gena Makhomed
I am use nginx web server (http://nginx.org/) with systemd.
systemd: Starting nginx - high performance web server...
systemd: Failed to read PID from file /var/run/nginx.pid: Invalid argument
systemd: Started nginx - high performance web server.
systemd: Starting nginx - high performance web server...
systemd: PID file /var/run/nginx.pid not readable (yet?) after start.
systemd: Started nginx - high performance web server

either the daemon needs to be fixed

Ok, but can you tell, what is wrong with daemon?

There is nothing wrong with nginx per se. I am using a nginx.service
https://git.pengutronix.de/cgit/ptxdist/plain/projectroot/usr/lib/systemd/system/nginx.service

BTW, in you nginx.service file line

ExecStartPre=/usr/sbin/nginx -t

is useless. It does not work as restart guard.

ExecRestartPre= in TODO but not released yet in systemd.

https://lists.freedesktop.org/archives/systemd-devel/2014-July/021642.html

Post by Clemens Gruber
If nginx was not built with --pid-path=/var/run/nginx.pid you have to
override it in your nginx.conf with the correct pid path, e.g.
pid /var/run/nginx.pid;

nginx is build with "--pid-path=/var/run/nginx.pid"
and "PIDFile=/var/run/nginx.pid" is in nginx.service file.

Post by Clemens Gruber
(Your error looks like nginx did create the pid file somewhere else and
systemd can't find it)

All ok with pid file, it located at /var/run/nginx.pid
and no directive pid defined in nginx.conf file.

Post by Clemens Gruber
Buildroot has a different approach. They add that pid config line, etc.
https://github.com/buildroot/buildroot/blob/master/package/nginx/nginx.service

I use official nginx builds from nginx site:
http://nginx.org/en/linux_packages.html#mainline
OS CentOS 7.4, nginx 1.13.7

I restart nginx service and see the same error:

systemd: Stopping nginx - high performance web server...
systemd: Starting nginx - high performance web server...
systemd: PID file /var/run/nginx.pid not readable (yet?) after start.
systemd: Started nginx - high performance web server.

# cat /var/run/nginx.pid
12488

# systemctl --version
systemd 219
+PAM +AUDIT +SELINUX +IMA -APPARMOR +SMACK +SYSVINIT +UTMP
+LIBCRYPTSETUP +GCRYPT +GNUTLS +ACL +XZ -LZ4 -SECCOMP +BLKID +ELFUTILS
+KMOD +IDN

[Clemens Gruber]

Post by Gena Makhomed
nginx is build with "--pid-path=/var/run/nginx.pid"
and "PIDFile=/var/run/nginx.pid" is in nginx.service file.

OK.

Post by Gena Makhomed
All ok with pid file, it located at /var/run/nginx.pid
and no directive pid defined in nginx.conf file.

That's odd. Can you try this on a newer systemd version? Yours is pretty
old. I'm on systemd 235 (ArchLinux).
Maybe systemd 219 behaved differently for forking services?

And just to be sure, it behaves the same if you add the
pid /var/run/nginx.pid;
Or
pid /run/nginx.pid;
line to your nginx.conf ?
(Maybe it does not follow symlinks?)

I don't think nginx is doing anything wrong. This is working fine for
many people with and without systemd. There is even a similar example on
the nginx website:
https://www.nginx.com/resources/wiki/start/topics/examples/systemd/

Cheers,
Clemens

[Gena Makhomed]

Can you try this on a newer systemd version? Yours is pretty old.
I'm on systemd 235 (ArchLinux).
Maybe systemd 219 behaved differently for forking services?

This is does not help, as I understand from the latest systemd sources
- this is common systemd behaviour and it is not changed in new version.

I don't think nginx is doing anything wrong. This is working fine for
many people with and without systemd. There is even a similar example on
https://www.nginx.com/resources/wiki/start/topics/examples/systemd/

From systemd developers point of view this is bug in nginx source code.
From nginx developers point of view this is may be bug in systemd code.

Systemd expects what parent nginx process exits only when pid file
with child process pid already exists.

https://www.freedesktop.org/software/systemd/man/systemd.service.html#Type=

Type=

If set to forking, it is expected that the process configured with
ExecStart= will call fork() as part of its start-up. The parent process
is expected to exit when start-up is complete and all communication
channels are set up. The child continues to run as the main daemon
process. This is the behavior of traditional UNIX daemons. If this
setting is used, it is recommended to also use the PIDFile= option, so
that systemd can identify the main process of the daemon. systemd will
proceed with starting follow-up units as soon as the parent process exits.

[Lennart Poettering]

Post by Gena Makhomed

Post by Reindl Harald

Post by Gena Makhomed
I am use nginx web server (http://nginx.org/) with systemd.
systemd: Starting nginx - high performance web server...
systemd: Failed to read PID from file /var/run/nginx.pid: Invalid argument
systemd: Started nginx - high performance web server.
systemd: Starting nginx - high performance web server...
systemd: PID file /var/run/nginx.pid not readable (yet?) after start.
systemd: Started nginx - high performance web server

either the daemon needs to be fixed

Ok, but can you tell, what is wrong with daemon?
http://hg.nginx.org/nginx/file/tip/src/os/unix/ngx_daemon.c

This is not how UNIX daemonization is supposed to work. The daemon is
supposed to fork twice: the first child is supposed to call
setsid() and then call fork() and then exit(), and the grandchild is
then supposed to be the main daemon process, so that:

a) it's definitely reparented to PID 1

b) It runs within its own setsid session, that has no leader

Lennart

[Lennart Poettering]

Post by Gena Makhomed
I am use nginx web server (http://nginx.org/) with systemd.
systemd: Starting nginx - high performance web server...
systemd: Failed to read PID from file /var/run/nginx.pid: Invalid argument
systemd: Started nginx - high performance web server.
systemd: Starting nginx - high performance web server...
systemd: PID file /var/run/nginx.pid not readable (yet?) after start.
systemd: Started nginx - high performance web server

either the daemon needs to be fixed or just remove the pid-file stuff from
the systemd-unit, systemd don't need it really

Well, that's not really true. systemd doesn't need it if there's only
a single process around at the moment the daemon parent process
exits. For many simpler daemons that is usually the case, but for
daemons that employ worker processes this might be different. I'd
always recommend people to use PIDFile= if they use Type=forking.

That said, Type=forking is nasty, and does a lot of stuff that is very
much pointless in a systemd environment. If you have a sympathetic
upstream I'd always recommend switching to Type=notify and dropping
PIDFile= in the process, but that requires that the daemon sends
sd_notify("READY=1") when it finished starting up.

Lennart

[Mantas Mikulėnas]

Post by Gena Makhomed
This is bug in nginx code? And this bug should be fixed in nginx?
But "daemon(); write_pidfile();" is common pattern
used by many services and even in library functions.

It may be common, but not necessarily correct. The parent process should
only exit *after* finishing all the preparations (i.e. something like
"fork(); write_pidfile(); exit()"), since systemd uses the exit as a signal
that the daemon is now "ready".

[Gena Makhomed]

Post by Mantas Mikulėnas

Post by Gena Makhomed
This is bug in nginx code? And this bug should be fixed in nginx?
But "daemon(); write_pidfile();" is common pattern
used by many services and even in library functions.

It may be common, but not necessarily correct. The parent process should
only exit *after* finishing all the preparations (i.e. something like
"fork(); write_pidfile(); exit()"), since systemd uses the exit as a signal
that the daemon is now "ready".

You are joking? Why you think that this pattern is not correct?

Is any systemd documentations exists, which requires from daemons
write pidfile of daemon process before exiting the parent process?

Does you see daemon() function from The GNU C Library (glibc)?
It exits from parent process immediately, without writing pid file.
Probably you should first fix function daemon() from glibc to meet
systemd requirements? And after this - rewrite all existing daemons?

Or may be it will be simpler to fix root cause of problem in systemd?

Common UNIX pattern is to perform a double fork when creating a daemon.
See Stevens' "Advanced Programming in the UNIX Environment" for details
https://www.amazon.com/Advanced-Programming-UNIX-Environment-3rd/dp/0321637739

Parent process can't easily get the PID of the grandchild process,
it is not possible in common case to write pidfile before exit parent.

# Fork a second child and exit immediately to prevent zombies. This
# causes the second child process to be orphaned, making the init
# process responsible for its cleanup. And, since the first child is
# a session leader without a controlling terminal, it's possible for
# it to acquire one by opening a terminal in the future (System V-
# based systems). This second fork guarantees that the child is no
# longer a session leader, preventing the daemon from ever acquiring
# a controlling terminal.

- https://stackoverflow.com/a/881434

[Reindl Harald]

Post by Gena Makhomed

Post by Mantas Mikulėnas

Post by Gena Makhomed
This is bug in nginx code? And this bug should be fixed in nginx?
But "daemon(); write_pidfile();" is common pattern
used by many services and even in library functions.

It may be common, but not necessarily correct. The parent process should
only exit *after* finishing all the preparations (i.e. something like
"fork(); write_pidfile(); exit()"), since systemd uses the exit as a signal
that the daemon is now "ready".

You are joking? Why you think that this pattern is not correct?

common sense

Post by Gena Makhomed
Is any systemd documentations exists, which requires from daemons
write pidfile of daemon process before exiting the parent process?
Does you see daemon() function from The GNU C Library (glibc)?
It exits from parent process immediately, without writing pid file.
Probably you should first fix function daemon() from glibc to meet
systemd requirements? And after this - rewrite all existing daemons?

well, if you want to tell the main-pid to systemd it's logical that
there must be a defined point when that file exists, in most cases you
don't need the PID stuff at all as i have explained multiple times

if you want so the daemon needs to write the pid file before telling
systemd "i am ready"

[Michael Chapman]

Post by Gena Makhomed

Post by Mantas Mikulėnas

Post by Gena Makhomed
This is bug in nginx code? And this bug should be fixed in nginx?
But "daemon(); write_pidfile();" is common pattern
used by many services and even in library functions.

It may be common, but not necessarily correct. The parent process should
only exit *after* finishing all the preparations (i.e. something like
"fork(); write_pidfile(); exit()"), since systemd uses the exit as a signal
that the daemon is now "ready".

You are joking? Why you think that this pattern is not correct?

Based on my understanding of the code, Type=forking services are not
deemed to be active until it sees _both_ the process has exited and the
PID file has appeared (so long as systemd is actually told about the PID
file, of course).

It should not matter whether a Type=forking service's initial process
exits first or writes its PID file first. If the PID file is missing when
the process exits, systemd will log a "PID file ... not readable (yet?)"
message, and it will keep the service in an "activating" state until it
actually becomes readable.

In short, either design should work correctly.

However, there is something to be said for only writing out the PID file
after the initial process has confirmed the service is operating
correctly. This is possible with fork() (and some kind of IPC or signal
between the child process and the original process) but not with daemon().
daemon() always exits successfully, which means other process managers
that don't PID files like systemd does would assume the process had
started successfully, even if the service's child process died straight
away.

Many other deficiencies with the BSD daemon() function are documented in
systemd's daemon(7) manpage.

I would suggest ignoring some other commenters suggestions that PID files
are unnecessary with systemd. Indeed, there's even a TODO item for a
Type=pid-file service that would, presumably, allow even non-forking
services to signal readiness by writing a PID file. I would find something
like that very useful.

[Gena Makhomed]

Post by Michael Chapman
Many other deficiencies with the BSD daemon() function are documented in
systemd's daemon(7) manpage.

Michael, thank you for reference to daemon(7) manpage,
this is exactly that I am looking for.

I will try to ask nginx developers for implementing
changes required by systemd from SysV Daemons.

Post by Michael Chapman
I would suggest ignoring some other commenters suggestions that PID
files are unnecessary with systemd. Indeed, there's even a TODO item for
a Type=pid-file service that would, presumably, allow even non-forking
services to signal readiness by writing a PID file. I would find
something like that very useful.

From TODO file "ExecRestartPre=" feature will be very useful
for protecting users which edit nginx or httpd configuration files
and restarts nginx or httpd to apply changes. If someone make error
in configuration file and restart service - nginx or httpd
will not start after stop and service will be not working.

"ExecRestartPre=" feature will help to protect from such situations:

https://lists.freedesktop.org/archives/systemd-devel/2014-July/021642.html

Record in TODO file exists, even issue on github exists:

https://github.com/systemd/systemd/issues/2175

with comment from nginx.org packages maintainer Konstantin Pavlov:

https://github.com/systemd/systemd/issues/2175#issuecomment-343481962

But as I understand, feature "ExecRestartPre=" will not be included
in the upcoming CentOS 8 / RHEL 8 system, because nobody
of systemd developers have time to implement this feature.

Or anybody of systemd developers has time to implement this feature?

[Colin Guthrie]

Post by Gena Makhomed

Post by Michael Chapman
Many other deficiencies with the BSD daemon() function are documented
in systemd's daemon(7) manpage.

Michael, thank you for reference to daemon(7) manpage,
this is exactly that I am looking for.
I will try to ask nginx developers for implementing
changes required by systemd from SysV Daemons.

Post by Michael Chapman
I would suggest ignoring some other commenters suggestions that PID
files are unnecessary with systemd. Indeed, there's even a TODO item
for a Type=pid-file service that would, presumably, allow even
non-forking services to signal readiness by writing a PID file. I
would find something like that very useful.

From TODO file "ExecRestartPre=" feature will be very useful
for protecting users which edit nginx or httpd configuration files
and restarts nginx or httpd to apply changes. If someone make error
in configuration file and restart service - nginx or httpd
will not start after stop and service will be not working.
https://lists.freedesktop.org/archives/systemd-devel/2014-July/021642.html
https://github.com/systemd/systemd/issues/2175
https://github.com/systemd/systemd/issues/2175#issuecomment-343481962
But as I understand, feature "ExecRestartPre=" will not be included
in the upcoming CentOS 8 / RHEL 8 system, because nobody
of systemd developers have time to implement this feature.
Or anybody of systemd developers has time to implement this feature?

I know this is somewhat tangential but in many of the cases you describe
here, a "reload" is more appropriate than a "restart". The config
parsing logic could either be specified in the systemd unit (just as one
of many ExecReload= entries) or could be handled internally in the
binary itself (presumably when the mainpid has received some signal).

That's not to take away from the usefulness of an ExecRestartPre=
directive of course, it's just that with nginx and httpd restarting is
not something you would ideally want to do in production unless you can
use full socket activation supported by systemd so as not to create a
small window when connections from clients are refused.

Col
--
Colin Guthrie
gmane(at)colin.guthr.ie
http://colin.guthr.ie/

Day Job:
Tribalogic Limited http://www.tribalogic.net/
Open Source:
Mageia Contributor http://www.mageia.org/
PulseAudio Hacker http://www.pulseaudio.org/
Trac Hacker http://trac.edgewall.org/

[Lennart Poettering]

Post by Gena Makhomed

Post by Mantas Mikulėnas

Post by Gena Makhomed
This is bug in nginx code? And this bug should be fixed in nginx?
But "daemon(); write_pidfile();" is common pattern
used by many services and even in library functions.

It may be common, but not necessarily correct. The parent process should
only exit *after* finishing all the preparations (i.e. something like
"fork(); write_pidfile(); exit()"), since systemd uses the exit as a signal
that the daemon is now "ready".

You are joking? Why you think that this pattern is not correct?

It doesn't work on SysV init either: "/etc/init.d/ngnx start ;
/etc/init.d/ngnx stop" is racy as it will leave ngnx running if it the
PID file isn't written by the time the stop command is used.

Post by Gena Makhomed
Is any systemd documentations exists, which requires from daemons
write pidfile of daemon process before exiting the parent process?

Yes:

https://www.freedesktop.org/software/systemd/man/daemon.html

Post by Gena Makhomed
Does you see daemon() function from The GNU C Library (glibc)?
It exits from parent process immediately, without writing pid file.
Probably you should first fix function daemon() from glibc to meet systemd
requirements? And after this - rewrite all existing daemons?

daemon() is hard to use properly, but it's possible: create a pipe()
first, then invoke daemon(). Then write the PID file in the daemon
process. Then close both sides of the pipe in the daemon process. In
the parent close the writing side, and read() from the reading
side. It will block until the daemon side closed its fds, and then get
EOF. And there, you got your synchronization done properly.

Post by Gena Makhomed
Or may be it will be simpler to fix root cause of problem in
systemd?

Well, I wish you good luck fixing SysV init scripts too. Before we
talk about "fixing systemd", let's talk how do you intend to make
"/etc/init.d/ngnx start ; /etc/init.d/ngnx stop" work correctly,
without races, without sometimes leaving ngnx running, if you don't
wait for the PID file to be written safely before exiting.

Lennart

[Lennart Poettering]

Post by Gena Makhomed
But nginx is forking daemon - when start process exited -
no warranty what pidfile already created by child process.
This is bug in nginx code? And this bug should be fixed in nginx?

Yes, and that's not just broken in the context of systemd, but on SysV
services, too. Think "/etc/init.d/nginx start ; /etc/init.d/nginx
stop" — if the PID file is not written fully by the time nginx forked
and the parent exited, then such a stop immediately following will
fail too...

Daemons need to either write the PID file in the parent, after forking
off the daemon, and before exiting, or the daemon process can do that,
but then there needs to be some form of synchronization that makes
sure that the parent doesn't exit before the daemon process finished
writing the PID file.

This is documented in daemon(7) btw:

https://www.freedesktop.org/software/systemd/man/daemon.html

Post by Gena Makhomed
But "daemon(); write_pidfile();" is common pattern
used by many services and even in library functions.

Yeah, and it's too simplistic unfortunately.

Post by Gena Makhomed
ExecStop=/bin/kill -s TERM $MAINPID

This line is redundant btw, systemd does that anyway.

Lennart