log core: Fix issue which could cause piped loggers to be orphaned

and never terminate after a graceful restart. PR 40651. [Joe Orton,
Ruediger Pluem]

log core: fix the new piped logger case where we couldn't connect
the replacement stderr logger's stderr to the NULL stdout stream.
Continue in this case, since the previous alternative of no error
logging at all (/dev/null) is far worse. [William Rowe]

disambiguate an error message to diagnose future error reports

Backport: 452431, 568326, 568322

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

diff --git a/CHANGES b/CHANGES
index de5cf7f42fb5fd4e8b9ba25b84ab003fb2b14c5a..a9b0518db0d7d9142a0ebf940b15d7b1d2161d03 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -1,6 +1,15 @@
                                                          -*- coding: utf-8 -*-
 Changes with Apache 2.0.61
 
+  *) log core: Fix issue which could cause piped loggers to be orphaned 
+     and never terminate after a graceful restart. PR 40651. [Joe Orton, 
+     Ruediger Pluem]
+
+  *) log core: fix the new piped logger case where we couldn't connect 
+     the replacement stderr logger's stderr to the NULL stdout stream.  
+     Continue in this case, since the previous alternative of no error 
+     logging at all (/dev/null) is far worse. [William Rowe]
+
   *) mpm_winnt: Prevent the parent-child pipe from leaking into other
      spawned processes, and ensure we have a /Device/null handle for
      stdout when running as-a-service.  [William Rowe]

diff --git a/STATUS b/STATUS
index 9add374818eb125ffeb3178134796f758271f503..51f230018f0e4211c66426e4e4a3ee4f99f99343 100644 (file)
--- a/STATUS
+++ b/STATUS
@@ -133,23 +133,6 @@ PATCHES PROPOSED TO BACKPORT FROM TRUNK:
          http://svn.apache.org/viewcvs.cgi?rev=102870&view=rev
        +1: wrowe, colm
 
-    * log core: Fix issue which could cause piped loggers to be orphaned 
-      and never terminate after a graceful restart. PR 40651. [Joe Orton, 
-      Ruediger Pluem]
-        http://svn.apache.org/viewvc?view=rev&revision=452431
-      log core: fix the new piped logger case where we couldn't connect 
-      the replacement stderr logger's stderr to the NULL stdout stream.  
-      Continue in this case, since the previous alternative of no error 
-      logging at all (/dev/null) is far worse. [William Rowe]
-        http://svn.apache.org/viewvc?view=rev&revision=568326
-      r452431 plus r568326; adjusted for 2.0 (note second iteration!);
-        http://people.apache.org/~wrowe/r568326-backport-2.0-r2.patch
-      disambiguate an error message to diagnose future error reports
-        http://svn.apache.org/viewvc?view=rev&revision=568322
-        +1: wrowe
-        rpluem says: +1 once r568446 is backported.
-        jim: agree with rpluem
-
     * main core: Emit errors during the initial apr_app_initialize()
       or apr_pool_create() (when apr-based error reporting is not ready).
       [William Rowe, Jeff Trawick]

diff --git a/server/log.c b/server/log.c
index 555104f9d08bfdaccc2d4c49fbdbcbe79168469f..a06eda7c140cbe785ebfbd93c109c38ba3c40e8f 100644 (file)
--- a/server/log.c
+++ b/server/log.c
@@ -230,7 +230,7 @@ static void log_child_errfn(apr_pool_t *pool, apr_status_t err,
 }
 
 static int log_child(apr_pool_t *p, const char *progname,
-                     apr_file_t **fpin)
+                     apr_file_t **fpin, int dummy_stderr)
 {
     /* Child process code for 'ErrorLog "|..."';
      * may want a common framework for this, since I expect it will
@@ -239,6 +239,7 @@ static int log_child(apr_pool_t *p, const char *progname,
     apr_status_t rc;
     apr_procattr_t *procattr;
     apr_proc_t *procnew;
+    apr_file_t *errfile;
 
     if (((rc = apr_procattr_create(&procattr, p)) == APR_SUCCESS)
         && ((rc = apr_procattr_io_set(procattr,
@@ -246,13 +247,20 @@ static int log_child(apr_pool_t *p, const char *progname,
                                       APR_NO_PIPE,
                                       APR_NO_PIPE)) == APR_SUCCESS)
         && ((rc = apr_procattr_error_check_set(procattr, 1)) == APR_SUCCESS)
-        && ((rc = apr_procattr_child_errfn_set(procattr, log_child_errfn)) == APR_SUCCESS)) {
+        && ((rc = apr_procattr_child_errfn_set(procattr, log_child_errfn))
+                == APR_SUCCESS)) {
         char **args;
         const char *pname;
 
         apr_tokenize_to_argv(progname, &args, p);
         pname = apr_pstrdup(p, args[0]);
         procnew = (apr_proc_t *)apr_pcalloc(p, sizeof(*procnew));
+
+        if (dummy_stderr) {
+            if ((rc = apr_file_open_stdout(&errfile, p)) == APR_SUCCESS)
+                rc = apr_procattr_child_err_set(procattr, errfile, NULL);
+        }
+
         rc = apr_proc_create(procnew, pname, (const char * const *)args,
                              NULL, procattr, p);
 
@@ -268,7 +276,7 @@ static int log_child(apr_pool_t *p, const char *progname,
     return rc;
 }
 
-static int open_error_log(server_rec *s, apr_pool_t *p)
+static int open_error_log(server_rec *s, int is_main, apr_pool_t *p)
 {
     const char *fname;
     int rc;
@@ -276,8 +284,11 @@ static int open_error_log(server_rec *s, apr_pool_t *p)
     if (*s->error_fname == '|') {
         apr_file_t *dummy = NULL;
 
-        /* This starts a new process... */
-        rc = log_child (p, s->error_fname + 1, &dummy);
+        /* Spawn a new child logger.  If this is the main server_rec,
+         * the new child must use a dummy stderr since the current
+         * stderr might be a pipe to the old logger.  Otherwise, the
+         * child inherits the parents stderr. */
+        rc = log_child(p, s->error_fname + 1, &dummy, is_main);
         if (rc != APR_SUCCESS) {
             ap_log_error(APLOG_MARK, APLOG_STARTUP, rc, NULL,
                          "Couldn't start ErrorLog process");
@@ -340,7 +351,7 @@ int ap_open_logs(apr_pool_t *pconf, apr_pool_t *p /* plog */,
 
     apr_pool_cleanup_register(p, NULL, clear_handle_list,
                               apr_pool_cleanup_null);
-    if (open_error_log(s_main, p) != OK) {
+    if (open_error_log(s_main, 1, p) != OK) {
         return DONE;
     }
 
@@ -379,7 +390,7 @@ int ap_open_logs(apr_pool_t *pconf, apr_pool_t *p /* plog */,
             }
 
             if (q == virt) {
-                if (open_error_log(virt, p) != OK) {
+                if (open_error_log(virt, 0, p) != OK) {
                     return DONE;
                 }
             }
@@ -949,7 +960,7 @@ AP_DECLARE(piped_log *) ap_open_piped_log(apr_pool_t *p, const char *program)
     apr_file_t *dummy = NULL;
     int rc;
 
-    rc = log_child(p, program, &dummy);
+    rc = log_child(p, program, &dummy, 0);
     if (rc != APR_SUCCESS) {
         ap_log_error(APLOG_MARK, APLOG_STARTUP, rc, NULL,
                      "Couldn't start piped log process");