On 6 November 2013 13:50, Steve Hay <steve.m....@googlemail.com> wrote:
> On 6 November 2013 12:27, Jeff Trawick <traw...@gmail.com> wrote:
>> On Wed, Nov 6, 2013 at 4:02 AM, Steve Hay <steve.m....@googlemail.com>
>> wrote:
>>>
>>> On 6 November 2013 00:48, Jeff Trawick <traw...@gmail.com> wrote:
>>> > Back to the httpd24threading branch:
>>> >
>>> > * modperl_interp_pool_select() has this notion of phase, which must
>>> > either
>>> > be startup or request context.
>>> > * It thinks it is startup only if the pool passed in is
>>> > s->process->pconf.
>>> > * Sometimes it is passed s->process->pool (parent of pconf), such as
>>> > from
>>> > perl_parse_require_line().
>>> > * perl_parse_require_line() can sometimes be called from request
>>> > context.
>>> > * When perl_parse_require_line() calls modperl_interp_pool_select(),
>>> > request
>>> > context can never be identified because perl_parse_require_line() never
>>> > passes in r->pool (which I guess would be cmd->pool).
>>> > * etc.
>>> >
>>> > This would seem to be the way to get the right pool to
>>> > modperl_interp_pool_select().
>>> >
>>> > Index: src/modules/perl/modperl_util.c
>>> > ===================================================================
>>> > --- src/modules/perl/modperl_util.c (revision 1539040)
>>> > +++ src/modules/perl/modperl_util.c (working copy)
>>> > @@ -989,7 +989,7 @@
>>> > int count;
>>> > void *key;
>>> > auth_callback *ab;
>>> > - MP_dINTERP_POOLa(cmd->server->process->pool, cmd->server);
>>> > + MP_dINTERP_POOLa(cmd->pool, cmd->server);
>>> >
>>> > if (global_authz_providers == NULL) {
>>> > MP_INTERP_PUTBACK(interp, aTHX);
>>> >
>>> > That still doesn't bring happiness (no interpreter returned, resulting
>>> > in a
>>> > crash trying to dereference interp).
>>> >
>>>
>>> I'm getting the same crash-on-startup behaviour now myself after a
>>> fresh rebuild of everything (now using httpd-2.4.6 and perl-5.19.5). I
>>> will look back over the changes made on the threading branch and/or my
>>> merges of them into the httpd24 branch. Hopefully the answer lies
>>> there somewhere. I'll be very grateful for any help I can get with
>>> this though -- I didn't do the original work on either of those
>>> branches...
>>
>>
>> With the "fix" above in place, modperl_init_vhost() seems to be the next
>> crucial code. We go down this path:
>>
>> if (base_server == s) {
>> MP_TRACE_i(MP_FUNC, "base server is not vhost, skipping %s",
>> vhost);
>> return OK;
>> }
>>
>> and fall through this FIXME in modperl_interp_pool_select():
>>
>> if (!scfg->mip) {
>> /* FIXME: We get here if global "server_rec" == s,
>> scfg->mip
>> * is not created then. I'm not sure if that's bug or
>> * bad/good design decicision. For now just return NULL.
>> */
>> return NULL;
>> }
>>
>> (Note: disabling the base_server == s check in modperl_init_vhost() brings
>> no happiness either, though perhaps it is a step in the right direction.)
>>
>> This path is new with httpd 2.4; 2.2 didn't have authz_providers.
>>
>> This seems to be a whack-a-mole issue. I'd expect that there is some easy
>> way to grab the interpreter for any arbitrary startup path, but I don't see
>> it. Maybe it is worthwhile seeing if we already went through some paths
>> where we were able to grab an interpreter.
>>
>
> The last change on the httpd24 branch (r1503193) is what added the
> FIXME above, but it also made a change in perl_parse_require_line()
> which I've lost in the course of merging the threading branch in: it
> made that function tolerant of modperl_interp_pool_select() returning
> NULL (which is exactly what happens in the FIXME case).
>
> If modperl_interp_pool_select() returns NULL then
> perl_parse_require_line() just returns NULL itself in r1503193, but in
> httpd24threading I've hidden the use of modperl_interp_pool_select()
> within the MP_dINTERP_POOLa() macro (as per the general style of
> changes in the threading branch), but that macro crashes if
> modperl_interp_pool_select() has returned NULL.
>
> The diff below makes that macro tolerant of
> modperl_interp_pool_select() returning NULL, and makes
> perl_parse_require_line() tolerant of interp ending up NULL, like it
> used to be in r1503193.
>
> With this diff in place (which includes your earlier change), the
> server now starts up for me and tests appear to be running as normal.
Oops! In my excitement I forgot the diff!:
Index: src/modules/perl/modperl_interp.h
===================================================================
--- src/modules/perl/modperl_interp.h (revision 1539262)
+++ src/modules/perl/modperl_interp.h (working copy)
@@ -68,9 +68,12 @@
#define MP_INTERP_POOLa(p, s) \
MP_TRACE_i(MP_FUNC, "selecting interp: p=%pp, s=%pp", (p), (s)); \
interp = modperl_interp_pool_select((p), (s)); \
- MP_TRACE_i(MP_FUNC, " --> got (0x%pp)->refcnt=%d", \
- interp, interp->refcnt); \
- aTHX = interp->perl
+ if (interp) { \
+ MP_TRACE_i(MP_FUNC, " --> got (0x%pp)->refcnt=%d", \
+ interp, interp->refcnt); \
+ aTHX = interp->perl; \
+ } \
+ NOOP
#define MP_dINTERP_POOLa(p, s) \
MP_dINTERP; \
Index: src/modules/perl/modperl_util.c
===================================================================
--- src/modules/perl/modperl_util.c (revision 1539262)
+++ src/modules/perl/modperl_util.c (working copy)
@@ -989,8 +989,11 @@
int count;
void *key;
auth_callback *ab;
- MP_dINTERP_POOLa(cmd->server->process->pool, cmd->server);
+ MP_dINTERP_POOLa(cmd->pool, cmd->server);
+ if (!interp)
+ return ret;
+
if (global_authz_providers == NULL) {
MP_INTERP_PUTBACK(interp, aTHX);
return ret;
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@perl.apache.org
For additional commands, e-mail: dev-h...@perl.apache.org
