Bug #2695
closed[PATCH] server.c: call ftruncate on pid file
Description
If the server has changed its uid or is running in a chroot
it may be unable to remove the pid file when it exits.
However, if it holds on to an open handle to the pid file
that has write permission, it will be able to truncate the
pid file to 0 bytes in length.
Most monitoring software recognizes a 0-length pid file
as indicating there is no process running.
Therefore always attempt to truncate the pid file before
trying to remove it so that it's not left containing the
pid of a process that is no longer running.
Patch file attached.
See also http://repo.or.cz/lighttpd/svnmirror/patches.git/commitdiff/2586bb88
Files
Updated by stbuehler about 9 years ago
- Target version changed from 1.4.x to 1.4.39
Sounds useful; I will need to have a closer look whether:
- it needs close-on-exec flag
- what happens in the max-worker spawning case
Updated by mackyle about 9 years ago
- File 0004-server.c-call-ftruncate-on-pid-file_patch.txt 0004-server.c-call-ftruncate-on-pid-file_patch.txt added
I tend to think of lighttpd as not forking, but you're right, it does and it needs close-on-exec.
I have attached an improved patch with close-on-exec and proper graceful shutdown handling (the old patch overlooked that).
Also, in adding the graceful shutdown truncating, I slid the graceful shutdown unlink pid-file code down a few lines to be outside the fd loop so that an attempt is not made to unlink the pid file multiple times producing spurious "unlink failed for: ... 2 No such file or directory" messages.
See also http://repo.or.cz/lighttpd/svnmirror/patches.git/commitdiff/3fdb9db0
As for the max-worker spawning case, the workers ignore the pid file, right? If not, perhaps they should have their srv->srvconf.pid_file cleared and pid_fd set to -1 immediately after the fork that creates them as a worker has no business mucking with the pid file.
Updated by mackyle about 9 years ago
- File 0004-server.c-call-ftruncate-on-pid-file_patch.txt 0004-server.c-call-ftruncate-on-pid-file_patch.txt added
This version of the patch addresses the max-worker spawning case by making the workers' parent responsible for removing the pid-file. After all, shouldn't the process whose pid is recorded in the pid-file be responsible for removing it?
See also http://repo.or.cz/lighttpd/svnmirror/patches.git/commitdiff/c63a20c1
Updated by stbuehler almost 9 years ago
- Target version changed from 1.4.39 to 1.4.40
Updated by gstrauss almost 9 years ago
@mackyle, would you please put the repeated code to ftruncate and unlink the pid file into a subroutine?
e.g. void pidfile_cleanup(server *srv, int pid_fd);
Updated by mackyle almost 9 years ago
- File 0004-server.c-call-ftruncate-on-pid-file_patch.txt 0004-server.c-call-ftruncate-on-pid-file_patch.txt added
Updated patch attached.
See also http://repo.or.cz/lighttpd/svnmirror/patches.git/commitdiff/5b22648e
Updated by gstrauss almost 9 years ago
The latest patch looks good to me. Thanks, @mackyle!
FYI: I'm not sure if @stbuehler prefers patches attached to these tickets or github pull requests. I have been doing both for my own requests with tested patches.
Updated by stbuehler almost 9 years ago
- Status changed from New to Fixed
- % Done changed from 0 to 100
Applied in changeset r3112.
Also available in: Atom