httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Joe Orton <>
Subject [PATCH] piped_log_spawn leaks an fd
Date Mon, 16 May 2005 20:58:38 GMT
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));

View raw message