From: William A. Rowe Jr Date: Tue, 28 Aug 2007 05:19:10 +0000 (+0000) Subject: log core: Fix issue which could cause piped loggers to be orphaned X-Git-Tag: 2.0.61~37 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=09bc7052f63c55b9dd4b782dd6ac8cd4114938c5;p=thirdparty%2Fapache%2Fhttpd.git 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 de5cf7f42fb..a9b0518db0d 100644 --- 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 9add374818e..51f230018f0 100644 --- 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 555104f9d08..a06eda7c140 100644 --- 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");