Backport applied

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

diff --git a/CHANGES b/CHANGES
index efefa8c45ce989b26299342ad72a221cbc733711..100898d42a34e61a3c9c719b9af306a74825a409 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -1,6 +1,10 @@
                                                          -*- coding: utf-8 -*-
 Changes with Apache 2.0.61
 
+  *) log core: ensure we use a special pool for stderr logging, so that
+     the stderr channel remains valid from the time plog is destroyed,
+     until the time the open_logs hook is called again.  [William Rowe]
+
   *) mod_ssl: Version reporting update; displays 'compiled against'
      Apache and build-time SSL Library versions at loglevel [info],
      while reporting the run-time SSL Library version in the server

diff --git a/STATUS b/STATUS
index 687d37ea917377ab4687f8af3c0ee9faaeef5123..9074b4f994988f7a3eb38ac5a12afb5385283d3b 100644 (file)
--- a/STATUS
+++ b/STATUS
@@ -116,14 +116,6 @@ PATCHES ACCEPTED TO BACKPORT FROM TRUNK:
   [ start all new proposals below, under PATCHES PROPOSED. ]
 
 
-    * log core: ensure we use a special pool for stderr logging, so that
-      the stderr channel remains valid from the time plog is destroyed,
-      until the time the open_logs hook is called again.  [William Rowe]
-        http://svn.apache.org/viewvc?view=rev&revision=569535
-      Backported to 2.0;
-        http://people.apache.org/~wrowe/r569535-backport-2.0-r2.patch
-      +1: wrowe, rpluem, jim
-
 PATCHES PROPOSED TO BACKPORT FROM TRUNK:
   [ please place SVN revisions from trunk here, so it is easy to
     identify exactly what the proposed changes are!  Add all new

diff --git a/server/log.c b/server/log.c
index a2a12cf5cebb5b515e29071f417a60ca08e13343..3eb56c69b2d9988bb2382e28512bae5eb45ef508 100644 (file)
--- a/server/log.c
+++ b/server/log.c
@@ -143,6 +143,8 @@ static const TRANS priorities[] = {
     {NULL,      -1},
 };
 
+static apr_pool_t *stderr_pool = NULL;
+
 static apr_file_t *stderr_log = NULL;
 
 /* track pipe handles to close in child process */
@@ -155,7 +157,7 @@ static read_handle_t *read_handles;
 
 /* clear_handle_list() is called when plog is cleared; at that
  * point we need to forget about our old list of pipe read
- * handles
+ * handles.  We let the plog cleanups close the actual pipes.
  */
 static apr_status_t clear_handle_list(void *v)
 {
@@ -209,12 +211,33 @@ AP_DECLARE(apr_status_t) ap_replace_stderr_log(apr_pool_t *p,
                      ap_server_argv0, fname);
         return rc;
     }
-    if ((rc = apr_file_open_stderr(&stderr_log, p)) == APR_SUCCESS) {
+    if (!stderr_pool) {
+        /* This is safe provided we revert it when we are finished.
+         * We don't manager the callers pool!
+         */
+        stderr_pool = p;
+    }
+    if ((rc = apr_file_open_stderr(&stderr_log, stderr_pool)) 
+            == APR_SUCCESS) {
         apr_file_flush(stderr_log);
-        if ((rc = apr_file_dup2(stderr_log, stderr_file, p)) == APR_SUCCESS) {
+        if ((rc = apr_file_dup2(stderr_log, stderr_file, stderr_pool)) 
+                == APR_SUCCESS) {
             apr_file_close(stderr_file);
+            /*
+             * You might ponder why stderr_pool should survive?
+             * The trouble is, stderr_pool may have s_main->error_log,
+             * so we aren't in a position to destory stderr_pool until
+             * the next recycle.  There's also an apparent bug which 
+             * is not; if some folk decided to call this function before 
+             * the core open error logs hook, this pool won't survive.
+             * Neither does the stderr logger, so this isn't a problem.
+             */
         }
     }
+    /* Revert, see above */
+    if (stderr_pool == p)
+        stderr_pool = NULL;
+
     if (rc != APR_SUCCESS) {
         ap_log_error(APLOG_MARK, APLOG_CRIT, rc, NULL,
                      "unable to replace stderr with error log file");
@@ -351,29 +374,54 @@ static int open_error_log(server_rec *s, int is_main, apr_pool_t *p)
 int ap_open_logs(apr_pool_t *pconf, apr_pool_t *p /* plog */, 
                  apr_pool_t *ptemp, server_rec *s_main)
 {
-    apr_status_t rc = APR_SUCCESS;
+    apr_pool_t *stderr_p;
     server_rec *virt, *q;
     int replace_stderr;
-    apr_file_t *errfile = NULL;
 
+
+    /* Register to throw away the read_handles list when we
+     * cleanup plog.  Upon fork() for the apache children,
+     * this read_handles list is closed so only the parent
+     * can relaunch a lost log child.  These read handles 
+     * are always closed on exec.
+     * We won't care what happens to our stderr log child 
+     * between log phases, so we don't mind losing stderr's 
+     * read_handle a little bit early.
+     */
     apr_pool_cleanup_register(p, NULL, clear_handle_list,
                               apr_pool_cleanup_null);
-    if (open_error_log(s_main, 1, p) != OK) {
+
+    /* HERE we need a stdout log that outlives plog.
+     * We *presume* the parent of plog is a process 
+     * or global pool which spans server restarts.
+     * Create our stderr_pool as a child of the plog's
+     * parent pool.
+     */
+    apr_pool_create(&stderr_p, apr_pool_parent_get(p));
+    apr_pool_tag(stderr_p, "stderr_pool");
+    
+    if (open_error_log(s_main, 1, stderr_p) != OK) {
         return DONE;
     }
 
     replace_stderr = 1;
     if (s_main->error_log) {
-        /* replace stderr with this new log */
+        apr_status_t rv;
+        
+        /* Replace existing stderr with new log. */
         apr_file_flush(s_main->error_log);
-        if ((rc = apr_file_open_stderr(&errfile, p)) == APR_SUCCESS) {
-            rc = apr_file_dup2(errfile, s_main->error_log, p);
-        }
-        if (rc != APR_SUCCESS) {
-            ap_log_error(APLOG_MARK, APLOG_CRIT, rc, s_main,
+        rv = apr_file_dup2(stderr_log, s_main->error_log, stderr_p);
+        if (rv != APR_SUCCESS) {
+            ap_log_error(APLOG_MARK, APLOG_CRIT, rv, s_main,
                          "unable to replace stderr with error_log");
         }
         else {
+            /* We are done with stderr_pool, close it, killing
+             * the previous generation's stderr logger
+             */
+            if (stderr_pool)
+                apr_pool_destroy(stderr_pool);
+            stderr_pool = stderr_p;
             replace_stderr = 0;
         }
     }