apr-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From John Sullivan <jo...@chiark.greenend.org.uk>
Subject Re: [PATCH] HUP on bug 45930 (CGI and signal masks)
Date Thu, 09 Sep 2010 00:37:32 GMT
On Wed, 08 Sep 2010 22:35:10 GMT, Nick Kew wrote:
> On Wed, 8 Sep 2010 13:28:36 +0100
> John Sullivan <johns@chiark.greenend.org.uk> wrote:
> 
> > Originally submitted with a patch nearly two years ago, this appears
> > to have languished. As far as I can tell nothing else has fixed the
> > bug in the meantime.
> > 
> > Could someone with check-in rights take a look at this: I think both
> > the problem and solution are straightforward.

Thanks for taking a look. To address your points:

> You submitted it under APR, but presented an HTTPD bug.
> That makes it less likely to get noticed (or taken seriously).

I originally submitted against the specific httpd version I reproduced
and tested the patch under. It was then reassigned to APR HEAD,
presumably by a httpd reviewer because that's where the necessary
code changes are.

> There are a couple of issues.
> 
> One: cross-platform issues: your patch is in /unix/ but references
> OS2.
> That flashes up "can't test so leave it" in me, and no doubt in other
> *X-oriented folks.  Only on replying did I look carefully enough to
> figure out it doesn't in fact affect OS2.

That is unfortunate. The OS2 reference only occurs in a #ifdef guard,
because exactly the same guard is placed around the code which blocks
the signals in the first place. Therefore the new code to reset the
signal mask will be called if and only if the mask has been set.

I have no idea why the guard is as it is, or whether its current form
is necessary, but to assume that it could be changed without a much
deeper understanding of the reasons it came about seems excessively
brave.

> Two: the problem description is using httpd mod_cgi with a threaded
> MPM.
> This is a configuration that is explicitly not recommended: mod_cgid

Maybe so, but I suspect that it is not uncommon operationally. Imagine
an existing code base using prefork with mod_cgi: due to performance
or resource consumption issues it is switched over to worker, a fairly
trivial configuration change, but reengineering the application code
is a much bigger change (and wasted if it turns out they need to
switch back.)

> (or fastcgi) are documented as preferable.  Easy to miss the
> underlying issue you're reporting!

Indeed, a rather rare bug in any case. But very confusing and
difficult to diagnose when it hits.

> Three: apr is a library, and HTTPD is just one user among many.
> A change that might have side-effects on something else belongs
> in trunk only.

I don't know the details of the APR or httpd process (or how you use
vcs tags as they relate to the release roadmap), so I didn't and
wouldn't suggest otherwise!

As long as it's making some forward progress to being integrated in
some form...

> Having said all that, bug me if I don't get around to reviewing it
> and either committing to trunk or raising objections in bugzilla.

Thanks,

John.
-- 

Mime
View raw message