Mailing-List: contact cvs-help@httpd.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@httpd.apache.org
Received-SPF: pass (hermes.apache.org: local policy)
Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Subject: svn commit: r452431 - in /httpd/httpd/trunk: CHANGES server/log.c
Date: Tue, 03 Oct 2006 11:40:30 -0000
To: cvs@httpd.apache.org
From: jorton@apache.org
Message-Id: <20061003114030.D9F651A981C@eris.apache.org>

Author: jorton
Date: Tue Oct  3 04:40:30 2006
New Revision: 452431

URL: http://svn.apache.org/viewvc?view=rev&rev=452431
Log:
When starting a new piped error logger for the main server, ensure
that the new child's stderr is not a pipe to an old piped logger:

* server/log.c (log_child): Add "dummy_stderr" parameter; if set,
duplicate stdout as the stderr for the child.
(open_error_log): Add "is_main" parameter; use dummy stderr for logger
for main server only.
(ap_open_logs, ap_open_piped_log): Adjust for new open_error_log()/
log_child() parameters.

PR: 40651
Submitted by: jorton, rpluem

Modified:
    httpd/httpd/trunk/CHANGES
    httpd/httpd/trunk/server/log.c

Modified: httpd/httpd/trunk/CHANGES
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/CHANGES?view=diff&rev=452431&r1=452430&r2=452431
==============================================================================
--- httpd/httpd/trunk/CHANGES [utf-8] (original)
+++ httpd/httpd/trunk/CHANGES [utf-8] Tue Oct  3 04:40:30 2006
@@ -2,6 +2,10 @@
 Changes with Apache 2.3.0
   [Remove entries to the current 2.0 and 2.2 section below, when backported]
 
+  *) Fix issue which could cause piped loggers to be orphaned and never
+     terminate after a graceful restart.  PR 40651.  [Joe Orton, 
+     Ruediger Pluem]
+
   *) mod_headers: support regexp-based editing of HTTP headers [Nick Kew]
 
   *) mod_echo: Fix precedence problem in if statement. PR 40658.

Modified: httpd/httpd/trunk/server/log.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/server/log.c?view=diff&rev=452431&r1=452430&r2=452431
==============================================================================
--- httpd/httpd/trunk/server/log.c (original)
+++ httpd/httpd/trunk/server/log.c Tue Oct  3 04:40:30 2006
@@ -225,8 +225,13 @@
                  "%s", description);
 }
 
+/* Create a child process running PROGNAME with a pipe connected to
+ * the childs stdin.  The write-end of the pipe will be placed in
+ * *FPIN on successful return.  If dummy_stderr is non-zero, the
+ * stderr for the child will be the same as the stdout of the parent.
+ * Otherwise the child will inherit the stderr from the parent. */
 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
@@ -235,6 +240,7 @@
     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_cmdtype_set(procattr,
@@ -244,7 +250,12 @@
                                       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)
+        && (!dummy_stderr 
+            || ((rc = apr_file_open_stdout(&errfile, p)) == APR_SUCCESS
+                && (rc = apr_procattr_child_err_set(procattr, 
+                                                    errfile, 
+                                                    errfile)) == APR_SUCCESS))) {
         char **args;
         const char *pname;
 
@@ -261,12 +272,21 @@
              * close_handle_in_child()
              */
         }
+
+        /* apr_procattr_child_err_set dups errfile twice: close the
+         * remaining "parent-side" copy (apr_proc_create closes the
+         * other). */
+        if (dummy_stderr) {
+            apr_file_close(procnew->err);
+        }
     }
 
     return rc;
 }
 
-static int open_error_log(server_rec *s, apr_pool_t *p)
+/* Open the error log for the given server_rec.  If IS_MAIN is
+ * non-zero, s is the main server. */
+static int open_error_log(server_rec *s, int is_main, apr_pool_t *p)
 {
     const char *fname;
     int rc;
@@ -274,8 +294,11 @@
     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");
@@ -338,7 +361,7 @@
 
     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;
     }
 
@@ -377,7 +400,7 @@
             }
 
             if (q == virt) {
-                if (open_error_log(virt, p) != OK) {
+                if (open_error_log(virt, 0, p) != OK) {
                     return DONE;
                 }
             }
@@ -955,7 +978,7 @@
     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");