Merge r170441, r170573, r170719 from trunk:

* server/log.c (piped_log_spawn): Return the APR error code,
as expected by piped_log_maintenance (and fixing "Unknown
error" messages in error path there).
(ap_open_piped_log): Expect an APR error code.

* server/log.c (ap_open_piped_log): Remove errno handling.

* server/log.c (piped_log_spawn): Don't leak an fd for each
invocation.

Reviewed by: jorton, trawick, wrowe

git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/branches/2.0.x@227329 13f79535-47bb-0310-9956-ffa450edef68

diff --git a/CHANGES b/CHANGES
index ae443f01997a4789dea540d167e7de61190bc35a..b0388c051376483357b56416ae84047f7e63a01d 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -1,6 +1,8 @@
                                                          -*- coding: utf-8 -*-
 Changes with Apache 2.0.55
 
+  *) Fix a file descriptor leak when starting piped loggers.  [Joe Orton]
+
   *) mod_ldap: Avoid segfaults when opening connections if using a version
      of OpenLDAP older than 2.2.21.  [Brad Nicholes]
 

diff --git a/STATUS b/STATUS
index ee01223fed69a7ab0f4a49fcad621b28bca4f9ef..66f0cad72b7a5012881b85f355eea60e4dd1708a 100644 (file)
--- a/STATUS
+++ b/STATUS
@@ -199,16 +199,6 @@ PATCHES ACCEPTED TO BACKPORT FROM TRUNK:
 
        +1: jim, pquerna, wrowe
 
-    *) Fix fd leak in piped logging code, fix error handling, and remove
-       dead errno handling.
-         http://svn.apache.org/viewcvs?rev=170441&view=rev
-         http://svn.apache.org/viewcvs?rev=170537&view=rev
-         http://svn.apache.org/viewcvs?rev=170719&view=rev
-       all-in-one patch incremental to the PR 26467 fix:
-         http://people.apache.org/~jorton/ap_pipedlog2.diff
-       +1: jorton, trawick, wrowe
-       [yes, I will write a CHANGES entry too]
-
     *) several changes to improve logging of connection-oriented errors, including
        ap_log_cerror() API (needs minor bump in addition to changes below)
          http://cvs.apache.org/viewcvs.cgi/httpd-2.0/server/core.c?r1=1.289&r2=1.291

diff --git a/server/log.c b/server/log.c
index bfec7a300395b123fcda44d409a552861e463587..10bdb566f38c2271d55d2781b78dc753d8be4507 100644 (file)
--- a/server/log.c
+++ b/server/log.c
@@ -763,9 +763,9 @@ AP_DECLARE(void) ap_log_assert(const char *szExp, const char *szFile,
 /* forward declaration */
 static void piped_log_maintenance(int reason, void *data, apr_wait_t status);
 
-static int piped_log_spawn(piped_log *pl)
+/* Spawn the piped logger process pl->program. */
+static apr_status_t piped_log_spawn(piped_log *pl)
 {
-    int rc = 0;
     apr_procattr_t *procattr;
     apr_proc_t *procnew = NULL;
     apr_status_t status;
@@ -783,7 +783,6 @@ static int piped_log_spawn(piped_log *pl)
         ap_log_error(APLOG_MARK, APLOG_STARTUP, 0, NULL,
                      "piped_log_spawn: unable to setup child process '%s': %s",
                      pl->program, apr_strerror(status, buf, sizeof(buf)));
-        rc = -1;
     }
     else {
         char **args;
@@ -797,7 +796,11 @@ static int piped_log_spawn(piped_log *pl)
 
         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));
@@ -808,11 +811,10 @@ static int piped_log_spawn(piped_log *pl)
             ap_log_error(APLOG_MARK, APLOG_STARTUP, 0, NULL,
                          "unable to start piped log program '%s': %s",
                          pl->program, apr_strerror(status, buf, sizeof(buf)));
-            rc = -1;
         }
     }
 
-    return rc;
+    return status;
 }
 
 
@@ -904,12 +906,10 @@ AP_DECLARE(piped_log *) ap_open_piped_log(apr_pool_t *p, const char *program)
     }
     apr_pool_cleanup_register(p, pl, piped_log_cleanup,
                               piped_log_cleanup_for_exec);
-    if (piped_log_spawn(pl) == -1) {
-        int save_errno = errno;
+    if (piped_log_spawn(pl) != APR_SUCCESS) {
         apr_pool_cleanup_kill(p, pl, piped_log_cleanup);
         apr_file_close(ap_piped_log_read_fd(pl));
         apr_file_close(ap_piped_log_write_fd(pl));
-        errno = save_errno;
         return NULL;
     }
     return pl;