On Fri, Nov 8, 2013 at 6:41 AM, <jkal...@apache.org> wrote: > Author: jkaluza > Date: Fri Nov 8 11:41:08 2013 > New Revision: 1539988 > > URL: http://svn.apache.org/r1539988 > Log: > Do not lose log messages with NULL server_rec when error log provider is > used. > > - set stderr_log to NULL after it is redirected to /dev/null > - use log provider of ap_server_conf in log_error_core when no server_rec > is provided and std_err_log is NULL >
I think this is resulting in a crash in my provider when ap_log_error() is called before open-logs (i.e., before my provider has had a chance to set up a destination for logging). This is the original 2.4 expectation regarding NULL server_rec IIUC: NULL is okay until logs are opened, at which point a NULL server_rec is a bug in the caller. But just pass ap_server_conf when you don't have a context-specific server_rec and core httpd will make sure it is always set when logging requires a server_rec. If at all practical, a NULL server_rec prior to setting up a configured provider shouldn't be treated differently than a NULL server_rec prior to setting a configured error log file/piped logger. My provider is getting a call on this path: #1 0x00007ffff1531a7b in elprov_error_log (info=0x7fffffffb340, handle=0x0, errstr=0x7fffffffb390 "AH00558: httpd: Could not reliably determine the server's fully qualified domain name, using 127.0.1.1. Set the 'ServerName' directive globally to suppress this message\n", len=169) at mod_elprov.c:38 #2 0x000000000046a3f9 in log_error_core (file=0x488f00 "util.c", line=2201, module_index=0, level=65, status=0, s=0x0, c=0x0, r=0x0, pool=0x6b6138, fmt=0x489618 "AH00558: %s: Could not reliably determine the server's fully qualified domain name, using %s. Set the 'ServerName' directive globally to suppress this message", args=0x7fffffffd408) at log.c:1259 #3 0x000000000046a709 in ap_log_perror_ (file=0x488f00 "util.c", line=2201, module_index=0, level=65, status=0, p=0x6b6138, fmt=0x489618 "AH00558: %s: Could not reliably determine the server's fully qualified domain name, using %s. Set the 'ServerName' directive globally to suppress this message") at log.c:1311 #4 0x00000000004335e7 in ap_get_local_host (a=0x6b6138) at util.c:2201 #5 0x000000000042d9f9 in ap_fini_vhost_config (p=0x6b6138, main_s=0x6df320) at vhost.c:565 #6 0x000000000042cac5 in main (argc=2, argv=0x7fffffffe0e8) at main.c:758 Maybe the handle should be checked (!= NULL) to see if there is a provider AND we've passed the critical point in the open-logs hook where the provider becomes usable. The call to the provider's init function in open-logs will abort startup if NULL is returned for the handle, so handle will never be NULL if the provider is initialized. > Modified: > httpd/httpd/trunk/server/log.c > > Modified: httpd/httpd/trunk/server/log.c > URL: > http://svn.apache.org/viewvc/httpd/httpd/trunk/server/log.c?rev=1539988&r1=1539987&r2=1539988&view=diff > > ============================================================================== > --- httpd/httpd/trunk/server/log.c (original) > +++ httpd/httpd/trunk/server/log.c Fri Nov 8 11:41:08 2013 > @@ -437,9 +437,12 @@ int ap_open_logs(apr_pool_t *pconf, apr_ > #define NULL_DEVICE "/dev/null" > #endif > > - if (replace_stderr && freopen(NULL_DEVICE, "w", stderr) == NULL) { > - ap_log_error(APLOG_MARK, APLOG_CRIT, errno, s_main, APLOGNO(00093) > - "unable to replace stderr with %s", NULL_DEVICE); > + if (replace_stderr) { > + if (freopen(NULL_DEVICE, "w", stderr) == NULL) { > + ap_log_error(APLOG_MARK, APLOG_CRIT, errno, s_main, > APLOGNO(00093) > + "unable to replace stderr with %s", NULL_DEVICE); > + } > + stderr_log = NULL; > } > > for (virt = s_main->next; virt; virt = virt->next) { > @@ -1034,6 +1037,8 @@ static void log_error_core(const char *f > const request_rec *rmain = NULL; > core_server_config *sconf = NULL; > ap_errorlog_info info; > + ap_errorlog_provider *errorlog_provider = NULL; > + void *errorlog_provider_handle = NULL; > > /* do we need to log once-per-req or once-per-conn info? */ > int log_conn_info = 0, log_req_info = 0; > @@ -1060,6 +1065,10 @@ static void log_error_core(const char *f > #endif > > logf = stderr_log; > + if (!logf && ap_server_conf && ap_server_conf->errorlog_provider) > { > + errorlog_provider = ap_server_conf->errorlog_provider; > + errorlog_provider_handle = > ap_server_conf->errorlog_provider_handle; > + } > } > else { > int configured_level = r ? ap_get_request_module_loglevel(r, > module_index) : > @@ -1078,6 +1087,9 @@ static void log_error_core(const char *f > logf = s->error_log; > } > > + errorlog_provider = s->errorlog_provider; > + errorlog_provider_handle = s->errorlog_provider_handle; > + > /* the faked server_rec from mod_cgid does not have > s->module_config */ > if (s->module_config) { > sconf = ap_get_core_module_config(s->module_config); > @@ -1106,6 +1118,14 @@ static void log_error_core(const char *f > } > } > > + if (!logf && !errorlog_provider) { > + /* There is no file to send the log message to (or it is > + * redirected to /dev/null and therefore any formating done below > + * would be lost anyway) and there is no log provider available, > so > + * we just return here. */ > + return; > + } > + > info.s = s; > info.c = c; > info.pool = pool; > @@ -1191,7 +1211,7 @@ static void log_error_core(const char *f > continue; > } > > - if (logf || (s->errorlog_provider->flags & > + if (logf || (errorlog_provider->flags & > AP_ERRORLOG_PROVIDER_ADD_EOL_STR)) { > /* Truncate for the terminator (as apr_snprintf does) */ > if (len > MAX_STRING_LEN - sizeof(APR_EOL_STR)) { > @@ -1205,8 +1225,8 @@ static void log_error_core(const char *f > write_logline(errstr, len, logf, level_and_mask); > } > else { > - s->errorlog_provider->writer(&info, > s->errorlog_provider_handle, > - errstr, len); > + errorlog_provider->writer(&info, errorlog_provider_handle, > + errstr, len); > } > > if (done) { > > > -- Born in Roswell... married an alien... http://emptyhammock.com/