On Wed, Jan 7, 2009 at 6:03 AM, Joe Orton <jor...@redhat.com> wrote: > On Tue, Jan 06, 2009 at 12:10:25PM -0600, William Rowe wrote: > > Would folks comment on Nathan's, Joe's and Stefan's work on > > > > https://issues.apache.org/bugzilla/show_bug.cgi?id=42829 > > > > and offer any comments on why this patch; > > > > https://issues.apache.org/bugzilla/attachment.cgi?id=22822 > > > > should not be applied to trunk/ and branches/2.2.x/ in time for > > the next releases? > > I never found the time to commit this for a bunch of reasons: > > 1) I never convinced myself that adding a bunch of complexity to > prefork, per Stefan's patch, was the right way to address this > > 2) I never worked out what impact the lack of check on the poll > descriptor event type (POLLERR etc) had on this, but that needs > resolving too > > 3) I hadn't checked whether worker et al had similar issues > > 4) I think that a simpler solution to this problem would be to add a > timeout (e.g. 30s) on the child pollset_poll() call so they are > guaranteed to pop at some point, but I haven't had time to try this or > work out whether that will suck in some other way.
Initial testing of your idea for a timeout was promising. I'll try to test it with graceful stop as well. Index: server/mpm/prefork/prefork.c =================================================================== --- server/mpm/prefork/prefork.c (revision 731724) +++ server/mpm/prefork/prefork.c (working copy) @@ -540,10 +540,12 @@ apr_int32_t numdesc; const apr_pollfd_t *pdesc; - /* timeout == -1 == wait forever */ - status = apr_pollset_poll(pollset, -1, &numdesc, &pdesc); + /* timeout == 10 seconds to avoid a hang at graceful restart/stop + * caused by the closing of sockets by the signal handler + */ + status = apr_pollset_poll(pollset, apr_time_from_sec(10), &numdesc, &pdesc); if (status != APR_SUCCESS) { - if (APR_STATUS_IS_EINTR(status)) { + if (APR_STATUS_IS_TIMEUP(status) || APR_STATUS_IS_EINTR(status)) { if (one_process && shutdown_pending) { return; }