Any reason why the patch below is not commited yet?
I stumbled across this problem today and reminded myself of this discussion,
but was astonished to find out that it never had been commited.
Regards
Rüdiger
Joe Orton wrote:
> As discussed previously; this patch stops killing piped loggers; except
> that prefork still kills them at shutdown and ungraceful restart since
> it signals the entire process group, so it's not entirely consistent.
>
> In all other cases (graceful restart with prefork; shutdown and both
> types of restart with worker and derivatives):
>
> - piped loggers are not explicitly killed. Once all the pipes to the
> piped logger are closed, the logger will read EOF from stdin and know to
> exit. This is subtle interface change for piped loggers.
>
> - piped loggers will survive across generations; you may have several
> sets running concurrently
>
> The effect is that during a graceful restart, log entries processed by
> lingering children are correctly handled by piped loggers. In all
> previous releases, they are just thrown away. I think the importance of
> this issue warrants the interface change for 2.2.
>
> Index: server/log.c
> ===================================================================
> --- server/log.c (revision 170945)
> +++ server/log.c (working copy)
> @@ -819,7 +819,6 @@
> NULL, procattr, pl->p);
>
> if (status == APR_SUCCESS) {
> - pl->pid = procnew;
> /* procnew->in was dup2'd from ap_piped_log_write_fd(pl);
> * since the original fd is still valid, close the copy to
> * avoid a leak. */
> @@ -851,9 +850,6 @@
> switch (reason) {
> case APR_OC_REASON_DEATH:
> case APR_OC_REASON_LOST:
> - 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);
> stats = ap_mpm_query(AP_MPMQ_MPM_STATE, &mpm_state);
> if (stats != APR_SUCCESS) {
> @@ -880,13 +876,12 @@
> case APR_OC_REASON_UNWRITABLE:
> /* We should not kill off the pipe here, since it may only be full.
> * If it really is locked, we should kill it off manually. */
> - break;
> + break;
>
> case APR_OC_REASON_RESTART:
> - if (pl->pid != NULL) {
> - apr_proc_kill(pl->pid, SIGTERM);
> - pl->pid = NULL;
> - }
> + /* At restart time, just forget about existing piped loggers.
> + * When the last child closes the pipe, the logger will read
> + * EOF and exit. */
> break;
>
> case APR_OC_REASON_UNREGISTER:
> @@ -895,27 +890,14 @@
> }
>
>
> -static apr_status_t piped_log_cleanup_for_exec(void *data)
> +static apr_status_t piped_log_cleanup(void *data)
> {
> piped_log *pl = data;
> -
> apr_file_close(ap_piped_log_read_fd(pl));
> apr_file_close(ap_piped_log_write_fd(pl));
> return APR_SUCCESS;
> }
>
> -
> -static apr_status_t piped_log_cleanup(void *data)
> -{
> - piped_log *pl = data;
> -
> - if (pl->pid != NULL) {
> - apr_proc_kill(pl->pid, SIGTERM);
> - }
> - return piped_log_cleanup_for_exec(data);
> -}
> -
> -
> AP_DECLARE(piped_log *) ap_open_piped_log(apr_pool_t *p, const char *program)
> {
> piped_log *pl;
> @@ -928,8 +910,7 @@
> &ap_piped_log_write_fd(pl), p) != APR_SUCCESS) {
> return NULL;
> }
> - apr_pool_cleanup_register(p, pl, piped_log_cleanup,
> - piped_log_cleanup_for_exec);
> + apr_pool_cleanup_register(p, pl, piped_log_cleanup, piped_log_cleanup);
> if (piped_log_spawn(pl) != APR_SUCCESS) {
> apr_pool_cleanup_kill(p, pl, piped_log_cleanup);
> apr_file_close(ap_piped_log_read_fd(pl));
>
>
