From: Jim Jagielski Date: Tue, 28 Aug 2007 14:59:35 +0000 (+0000) Subject: use a special pool for stderr logging X-Git-Tag: 2.2.6~90 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=58bf0311bb40aff636787f8268cc76bccf90f56f;p=thirdparty%2Fapache%2Fhttpd.git use a special pool for stderr logging git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/branches/2.2.x@570451 13f79535-47bb-0310-9956-ffa450edef68 --- diff --git a/CHANGES b/CHANGES index 7bc4fb94b73..aa5cf68f17e 100644 --- a/CHANGES +++ b/CHANGES @@ -1,6 +1,10 @@ -*- coding: utf-8 -*- Changes with Apache 2.2.6 + *) 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_negotiation: preserve Query String in resolving a type map PR 33112 [Jørgen Thomsen , Nick Kew] diff --git a/STATUS b/STATUS index a056e18896b..45b77870fec 100644 --- a/STATUS +++ b/STATUS @@ -79,14 +79,6 @@ RELEASE SHOWSTOPPERS: 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.2; - http://people.apache.org/~wrowe/r569535-backport-2.2-r2.patch - +1: wrowe, jim, rpluem - PATCHES PROPOSED TO BACKPORT FROM TRUNK: diff --git a/server/log.c b/server/log.c index 43661591d6d..14b4fdc21f1 100644 --- a/server/log.c +++ b/server/log.c @@ -139,6 +139,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 */ @@ -151,7 +153,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) { @@ -205,12 +207,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"); @@ -349,29 +372,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; } }