From: Rainer Jung Date: Sun, 18 Jan 2009 18:19:45 +0000 (+0000) Subject: Piped error loggers should use the reliable pipes, X-Git-Tag: 2.3.2~148 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=288ac7728a198ec86241262abb4165da971f71be;p=thirdparty%2Fapache%2Fhttpd.git Piped error loggers should use the reliable pipes, i.e. they should be automatically restarted when they die similar to what happens for access loggers. Patch makes error loggers use the same code path as access loggers. Side effect: patch adds ap_server_root_relative() to the error logger path before spawning. Reviewed by R. Pluem. Patch needs to be tested on Windows. git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/trunk@735516 13f79535-47bb-0310-9956-ffa450edef68 --- diff --git a/server/log.c b/server/log.c index 2c6d688f3a6..5de7bb89fc2 100644 --- a/server/log.c +++ b/server/log.c @@ -141,6 +141,14 @@ static apr_pool_t *stderr_pool = NULL; static apr_file_t *stderr_log = NULL; +/* wrap a piped_log* with additional info + * about special stderr treatment used for main + * error logger */ +typedef struct piped_log_wrapper { + piped_log *pl; + int special_stderr; +} piped_log_wrapper; + /* track pipe handles to close in child process */ typedef struct read_handle_t { struct read_handle_t *next; @@ -257,6 +265,7 @@ static void log_child_errfn(apr_pool_t *pool, apr_status_t err, "%s", description); } +#ifndef AP_HAVE_RELIABLE_PIPED_LOGS /* 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 @@ -310,6 +319,11 @@ static int log_child(apr_pool_t *p, const char *progname, return rc; } +#endif + +/* forward declaration */ +static piped_log* piped_log_open(apr_pool_t *p, const char *program, + int special_stderr); /* Open the error log for the given server_rec. If IS_MAIN is * non-zero, s is the main server. */ @@ -319,20 +333,28 @@ static int open_error_log(server_rec *s, int is_main, apr_pool_t *p) int rc; if (*s->error_fname == '|') { - apr_file_t *dummy = NULL; + piped_log *pl; + fname = ap_server_root_relative(p, s->error_fname + 1); + + if (!fname) { + ap_log_error(APLOG_MARK, APLOG_STARTUP, APR_EBADPATH, NULL, + "%s: Invalid error log path '%s'.", + ap_server_argv0, s->error_fname); + return NULL; + } /* 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"); + pl = piped_log_open(p, fname, is_main); + if (pl == NULL) { + ap_log_error(APLOG_MARK, APLOG_STARTUP, 0, NULL, + "%s: Couldn't start ErrorLog process '%s'.", + ap_server_argv0, fname); return DONE; } - - s->error_log = dummy; + s->error_log = ap_piped_log_write_fd(pl); } #ifdef HAVE_SYSLOG @@ -361,7 +383,7 @@ static int open_error_log(server_rec *s, int is_main, apr_pool_t *p) fname = ap_server_root_relative(p, s->error_fname); if (!fname) { ap_log_error(APLOG_MARK, APLOG_STARTUP, APR_EBADPATH, NULL, - "%s: Invalid error log path %s.", + "%s: Invalid error log path '%s'.", ap_server_argv0, s->error_fname); return DONE; } @@ -369,7 +391,7 @@ static int open_error_log(server_rec *s, int is_main, apr_pool_t *p) APR_APPEND | APR_WRITE | APR_CREATE | APR_LARGEFILE, APR_OS_DEFAULT, p)) != APR_SUCCESS) { ap_log_error(APLOG_MARK, APLOG_STARTUP, rc, NULL, - "%s: could not open error log file %s.", + "%s: could not open error log file '%s'.", ap_server_argv0, fname); return DONE; } @@ -406,7 +428,7 @@ int ap_open_logs(apr_pool_t *pconf, apr_pool_t *p /* plog */, */ 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; } @@ -414,7 +436,7 @@ int ap_open_logs(apr_pool_t *pconf, apr_pool_t *p /* plog */, replace_stderr = 1; if (s_main->error_log) { apr_status_t rv; - + /* Replace existing stderr with new log. */ apr_file_flush(s_main->error_log); rv = apr_file_dup2(stderr_log, s_main->error_log, stderr_p); @@ -868,16 +890,23 @@ AP_DECLARE(void) ap_log_assert(const char *szExp, const char *szFile, /* piped log support */ +AP_DECLARE(piped_log *) ap_open_piped_log(apr_pool_t *p, const char *program) +{ + return piped_log_open(p, program, 0); +} + #ifdef AP_HAVE_RELIABLE_PIPED_LOGS /* forward declaration */ static void piped_log_maintenance(int reason, void *data, apr_wait_t status); /* Spawn the piped logger process pl->program. */ -static apr_status_t piped_log_spawn(piped_log *pl) +static apr_status_t piped_log_spawn(piped_log_wrapper *plw) { apr_procattr_t *procattr; apr_proc_t *procnew = NULL; + apr_file_t *errfile; apr_status_t status; + piped_log *pl = plw->pl; if (((status = apr_procattr_create(&procattr, pl->p)) != APR_SUCCESS) || ((status = apr_procattr_cmdtype_set(procattr, @@ -886,6 +915,14 @@ static apr_status_t piped_log_spawn(piped_log *pl) ap_piped_log_read_fd(pl), ap_piped_log_write_fd(pl))) != APR_SUCCESS) || + /* If special_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. */ + (plw->special_stderr && (status = apr_file_open_stdout(&errfile, pl->p)) + != APR_SUCCESS) || + (plw->special_stderr && (status = apr_procattr_child_err_set(procattr, + errfile, NULL)) + != APR_SUCCESS) || ((status = apr_procattr_child_errfn_set(procattr, log_child_errfn)) != APR_SUCCESS) || ((status = apr_procattr_error_check_set(procattr, 1)) != APR_SUCCESS)) { @@ -895,6 +932,7 @@ static apr_status_t piped_log_spawn(piped_log *pl) "piped_log_spawn: unable to setup child process '%s': %s", pl->program, apr_strerror(status, buf, sizeof(buf))); } + else { char **args; const char *pname; @@ -912,7 +950,7 @@ static apr_status_t piped_log_spawn(piped_log *pl) * avoid a leak. */ apr_file_close(procnew->in); procnew->in = NULL; - apr_proc_other_child_register(procnew, piped_log_maintenance, pl, + apr_proc_other_child_register(procnew, piped_log_maintenance, plw, ap_piped_log_write_fd(pl), pl->p); close_handle_in_child(pl->p, ap_piped_log_read_fd(pl)); } @@ -931,7 +969,8 @@ static apr_status_t piped_log_spawn(piped_log *pl) static void piped_log_maintenance(int reason, void *data, apr_wait_t status) { - piped_log *pl = data; + piped_log_wrapper *plw = data; + piped_log *pl = plw->pl; apr_status_t stats; int mpm_state; @@ -941,7 +980,7 @@ static void piped_log_maintenance(int reason, void *data, apr_wait_t status) pl->pid = NULL; /* in case we don't get it going again, this * tells other logic not to try to kill it */ - apr_proc_other_child_unregister(pl); + apr_proc_other_child_unregister(plw); stats = ap_mpm_query(AP_MPMQ_MPM_STATE, &mpm_state); if (stats != APR_SUCCESS) { ap_log_error(APLOG_MARK, APLOG_STARTUP, 0, NULL, @@ -953,13 +992,26 @@ static void piped_log_maintenance(int reason, void *data, apr_wait_t status) ap_log_error(APLOG_MARK, APLOG_STARTUP, 0, NULL, "piped log program '%s' failed unexpectedly", pl->program); - if ((stats = piped_log_spawn(pl)) != APR_SUCCESS) { + + /* If special_stderr is non-zero, we need to get the write + * end of the pipe back from stderr before spawning. */ + if ((plw->special_stderr && (stats = apr_file_dup(&ap_piped_log_write_fd(pl), + stderr_log, pl->p)) + != APR_SUCCESS) || + ((stats = piped_log_spawn(plw)) != APR_SUCCESS) || + /* If special_stderr is non-zero, we need to close the write + * end of the pipe after spawning, because we write to it via + * stderr. */ + (plw->special_stderr && (stats = apr_file_close(ap_piped_log_write_fd(pl))) + != APR_SUCCESS)) { + + char buf[120]; + /* what can we do? This could be the error log we're having * problems opening up... */ - char buf[120]; ap_log_error(APLOG_MARK, APLOG_STARTUP, 0, NULL, "piped_log_maintenance: unable to respawn '%s': %s", - pl->program, apr_strerror(stats, buf, sizeof(buf))); + pl->program, apr_strerror(stats, buf, sizeof(buf))); } } break; @@ -1003,14 +1055,19 @@ static apr_status_t piped_log_cleanup(void *data) } -AP_DECLARE(piped_log *) ap_open_piped_log(apr_pool_t *p, const char *program) +static piped_log* piped_log_open(apr_pool_t *p, const char *program, + int special_stderr) { piped_log *pl; + piped_log_wrapper *plw; pl = apr_palloc(p, sizeof (*pl)); pl->p = p; pl->program = apr_pstrdup(p, program); pl->pid = NULL; + plw = apr_palloc(p, sizeof (*plw)); + plw->special_stderr = special_stderr; + plw->pl = pl; if (apr_file_pipe_create_ex(&ap_piped_log_read_fd(pl), &ap_piped_log_write_fd(pl), APR_FULL_BLOCK, p) != APR_SUCCESS) { @@ -1018,7 +1075,7 @@ AP_DECLARE(piped_log *) ap_open_piped_log(apr_pool_t *p, const char *program) } apr_pool_cleanup_register(p, pl, piped_log_cleanup, piped_log_cleanup_for_exec); - if (piped_log_spawn(pl) != APR_SUCCESS) { + if (piped_log_spawn(plw) != APR_SUCCESS) { apr_pool_cleanup_kill(p, pl, piped_log_cleanup); apr_file_close(ap_piped_log_read_fd(pl)); apr_file_close(ap_piped_log_write_fd(pl)); @@ -1037,16 +1094,18 @@ static apr_status_t piped_log_cleanup(void *data) return APR_SUCCESS; } -AP_DECLARE(piped_log *) ap_open_piped_log(apr_pool_t *p, const char *program) +static piped_log* piped_log_open(apr_pool_t *p, const char *program, + int special_stderr) { piped_log *pl; apr_file_t *dummy = NULL; int rc; - rc = log_child(p, program, &dummy, 0); + rc = log_child(p, program, &dummy, special_stderr); if (rc != APR_SUCCESS) { ap_log_error(APLOG_MARK, APLOG_STARTUP, rc, NULL, - "Couldn't start piped log process"); + "Couldn't start piped log process for '%s'", + (program == NULL) ? "NULL" : program); return NULL; } @@ -1072,4 +1131,3 @@ AP_IMPLEMENT_HOOK_VOID(error_log, const request_rec *r, apr_pool_t *pool, const char *errstr), (file, line, level, status, s, r, pool, errstr)) -