On Wed, Jan 7, 2009 at 6:03 AM, Joe Orton <[email protected]> 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;
}
