It's the month of the piped loggers... there's another issue in this
code (just had the report this morning, by coincidence), that each time
a piped logger is spawned (or respawned), an fd is leaked.
Without the fix to APR I committed earlier, this can cause the parent to
segfault if enough piped loggers die and are respawned that the ulimit
is reached and pipe() starts failing: set up ~100-odd piped loggers, and
do "killall rotatelogs" a handful of times to reproduce.
The fd use goes like this, by my reckoning:
1. ap_open_piped_log
-> creates pipe P1(R,W)
2. piped_log_spawn calls apr_procattr_child_in_set
-> creates pipe P2(R,W)
-> dup2's P1(R,W) into P2(R,W)
3. piped_log_spawn calls apr_proc_create
-> spawns child then closes P2(R)
4. piped_log_spawn does:
P1(W) <- P2(W), aka ap_piped_log_write_fd(pl) = procnew->in;
the reference to P1(W) is now lost and that fd was not closed; and for
little gain, since P1(W) and P2(W) are equivalent anyway.
I think the correct fix is to throw away P2(W) to avoid the leak; patch
below is tested to avoid the leak across respawns of unexpectedly killed
piped loggers; am I missing anything?
Index: server/log.c
===================================================================
--- server/log.c (revision 170441)
+++ server/log.c (working copy)
@@ -820,7 +820,11 @@
if (status == APR_SUCCESS) {
pl->pid = procnew;
- ap_piped_log_write_fd(pl) = procnew->in;
+ /* procnew->in was dup2'd from ap_piped_log_write_fd(pl);
+ * since the original fd is still valid, close the copy to
+ * avoid a leak. */
+ apr_file_close(procnew->in);
+ procnew->in = NULL;
apr_proc_other_child_register(procnew, piped_log_maintenance, pl,
ap_piped_log_write_fd(pl), pl->p);
close_handle_in_child(pl->p, ap_piped_log_read_fd(pl));
