You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@perl.apache.org by Geoffrey Young <ge...@modperlcookbook.org> on 2004/02/27 18:17:48 UTC

dynamic request hook ordering (Win32 testers needed :)

hi all

ok, I spent some time and was able to get configuration-based dynamic hook
ordering working.

the attached patch adds a PerlHook*Handler directive for each request phase
(minus the PerlResponseHandler).  so, you would have a configuration like this

  PerlHookTransHandler Last

valid values correspond to APR_HOOK* values: ReallyFirst, First, Middle,
Last, ReallyLast.

the directives (and effects) are global, so they are not allowed in vhosts -
you get one shot, just like you did with ClearModuleList/AddModule in 1.3.

two things of note:

  1 - I really need someone on Win32 to give this a whirl.  there's some
code in there (swiped from mod_info) that is Win32 specific so it needs to
be excercised.  running both the mod_perl test suite as well as the attached
tests would be greatly appreciated.

  2 - on a parallel with this specific feature, I noticed that mod_perl
hooks PerlOptions +GlobalRequest logic in post-read-request and
header-parser phases, via RUN_FIRST.  since we moved user-defined
PerlInitHandler logic to RUN_REALLY_FIRST, its possible that user-defined
Perl handlers will run _before_ mod_perl gets the chance to insert its
global logic.  I'm not sure if this is a bad thing, but it sounds bad.  so,
I created a MODPERL_HOOK_REALLY_FIRST define (-20) to put the +GlobalRequest
logic really, really first for those two phases.  please review.

--Geoff

Re: dynamic request hook ordering (Win32 testers needed :)

Posted by Geoffrey Young <ge...@modperlcookbook.org>.
>> the whole idea of static C variables/structs and threaded mpms really,
>> really confuses me.  but I guess it doesn't matter, since it's not a
>> per-request thing and all this happens before ap_mpm_run is called
>> anyway.
> 
> 
> You don't have any threads till the child_init phase, so there is no
> problem reading/writing static variables without any locking.

ok, I'll give it a whirl then.

> of course
> in this particular case we assume that this hook stuff won't be accessed
> at request time. I'd even go and NULL all the static things when you are
> done, just in case someone will attempt to use them.

ok.  thanks for the feedback.

> Yes, I thought of that too. I've moved all the perl clutter (most of it)
> to modperl_perl.c. So how about creating modperl_apache.c and plant it
> there?

modperl_apache.c it is.

I'll post another round on monday.  but the framework is pretty much the
same - it's just minor shuffling at this point, so Win32 testers still
appreciated :)

--Geoff


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org


Re: dynamic request hook ordering (Win32 testers needed :)

Posted by Stas Bekman <st...@stason.org>.
Geoffrey Young wrote:
>>>+/* we need to hook a few internal things before APR_HOOK_REALLY_FIRST */
>>>+#define MODPERL_HOOK_REALLY_FIRST (-20)
>>
>>
>>The name is confusing, since APR_REALLY_FIRST is -10 and you kept the
>>rest of the MODPERL_HOOK_ names matching . Call it REALLY_REALLY_FIRST?
>>or anything else which is not _REALLY_FIRST?
> 
> 
> I was leaning toward MODPERL_HOOK_REALLY_REALLY_FIRST at first, so ok.
> 
> 
>>please s/order/hook_order/ or similar, order on its own is too vague of
>>a name.
> 
> 
> well, the rest are vague too, but ok :)

I wasn't participating in selecting those names. In any case that's not the 
reason to make things better.

>>I think it's a waste. If it's used only by the main server and relevant
>>only at the startup, just use some static variable and don't blow the
>>size of scfg for vhosts.
> 
> 
> the whole idea of static C variables/structs and threaded mpms really,
> really confuses me.  but I guess it doesn't matter, since it's not a
> per-request thing and all this happens before ap_mpm_run is called anyway.

You don't have any threads till the child_init phase, so there is no problem 
reading/writing static variables without any locking. of course in this 
particular case we assume that this hook stuff won't be accessed at request 
time. I'd even go and NULL all the static things when you are done, just in 
case someone will attempt to use them.

>>The whole sorting code doesn't belong to modperl_util.c, IMHO. It's a
>>one-time use code and should be static in where it is used, i.e. mod_perl.c
>>
> 
> 
> I started out doing exactly that.  however, I didn't feel as though clogging
> up mod_perl.c with this little feature was worth it - I'd be adding, what,
> 200 lines of obscure-feature code to the main module?  so, I moved it over
> to modperl_util.c (after also debating modperl_config.c).
> 
> but if you think it's better in mod_perl.c that's fine with me.  I was just
> trying to keep that as clutter-free as possible.

Yes, I thought of that too. I've moved all the perl clutter (most of it) to 
modperl_perl.c. So how about creating modperl_apache.c and plant it there?

We definitely don't want to clutter modperl_util.c since that file should 
hopefully contain only functions that are used more than once across different 
files.

Also I still want to get the concept of public C API formed, so I'd love to 
see modperl_util_public.c or something, to have functions that we don't change 
once 2.0 is released and they could be used from XS extensions outside 
mod_perl core.

>>And finally, if there are already two modules who need to copy so much
>>cruft to resort hooks, perhaps it should become an Apache API, so that
>>you don't have to worry about win32 users and what not. This stuff is
>>really not urgent to put in, IMHO. So if it gets added in 2.0.49 that's
>>still cool with us, no?
> 
> 
> yeah, I tried to see how to abstract some of the stuff out, but really it's
> only the hook_lookup stuff that is duplicated.  everyone else would have
> their own needs.  little bits of logic here and there are the same, but
> mod_perl and mod_info both have their exceptional cases.

OK, then.

__________________________________________________________________
Stas Bekman            JAm_pH ------> Just Another mod_perl Hacker
http://stason.org/     mod_perl Guide ---> http://perl.apache.org
mailto:stas@stason.org http://use.perl.org http://apacheweek.com
http://modperlbook.org http://apache.org   http://ticketmaster.com

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org


Re: dynamic request hook ordering (Win32 testers needed :)

Posted by Geoffrey Young <ge...@modperlcookbook.org>.
>> +/* we need to hook a few internal things before APR_HOOK_REALLY_FIRST */
>> +#define MODPERL_HOOK_REALLY_FIRST (-20)
> 
> 
> The name is confusing, since APR_REALLY_FIRST is -10 and you kept the
> rest of the MODPERL_HOOK_ names matching . Call it REALLY_REALLY_FIRST?
> or anything else which is not _REALLY_FIRST?

I was leaning toward MODPERL_HOOK_REALLY_REALLY_FIRST at first, so ok.

> please s/order/hook_order/ or similar, order on its own is too vague of
> a name.

well, the rest are vague too, but ok :)

> I think it's a waste. If it's used only by the main server and relevant
> only at the startup, just use some static variable and don't blow the
> size of scfg for vhosts.

the whole idea of static C variables/structs and threaded mpms really,
really confuses me.  but I guess it doesn't matter, since it's not a
per-request thing and all this happens before ap_mpm_run is called anyway.

>> +
>> +/* from here down is support for dynamic hook ordering.  this is mostly
>> + * stolen from mod_info.c, so see also the logic and descriptions there.
>> + */
> 
> 
> That's a bad idea to point to description in another module. What
> happens if it gets nuked/rewritten and it won't match the code below any
> longer. Just copy it over while it's correct.

ok, I guess so.

> 
> The whole sorting code doesn't belong to modperl_util.c, IMHO. It's a
> one-time use code and should be static in where it is used, i.e. mod_perl.c
> 

I started out doing exactly that.  however, I didn't feel as though clogging
up mod_perl.c with this little feature was worth it - I'd be adding, what,
200 lines of obscure-feature code to the main module?  so, I moved it over
to modperl_util.c (after also debating modperl_config.c).

but if you think it's better in mod_perl.c that's fine with me.  I was just
trying to keep that as clutter-free as possible.

> And finally, if there are already two modules who need to copy so much
> cruft to resort hooks, perhaps it should become an Apache API, so that
> you don't have to worry about win32 users and what not. This stuff is
> really not urgent to put in, IMHO. So if it gets added in 2.0.49 that's
> still cool with us, no?

yeah, I tried to see how to abstract some of the stuff out, but really it's
only the hook_lookup stuff that is duplicated.  everyone else would have
their own needs.  little bits of logic here and there are the same, but
mod_perl and mod_info both have their exceptional cases.

--Geoff


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org


Re: dynamic request hook ordering (Win32 testers needed :)

Posted by Stas Bekman <st...@stason.org>.
Geoffrey Young wrote:
> hi all
> 
> ok, I spent some time and was able to get configuration-based dynamic hook
> ordering working.
> 
> the attached patch adds a PerlHook*Handler directive for each request phase
> (minus the PerlResponseHandler).  so, you would have a configuration like this
> 
>   PerlHookTransHandler Last
> 
> valid values correspond to APR_HOOK* values: ReallyFirst, First, Middle,
> Last, ReallyLast.
> 
> the directives (and effects) are global, so they are not allowed in vhosts -
> you get one shot, just like you did with ClearModuleList/AddModule in 1.3.
> 
> two things of note:
> 
>   1 - I really need someone on Win32 to give this a whirl.  there's some
> code in there (swiped from mod_info) that is Win32 specific so it needs to
> be excercised.  running both the mod_perl test suite as well as the attached
> tests would be greatly appreciated.
> 
>   2 - on a parallel with this specific feature, I noticed that mod_perl
> hooks PerlOptions +GlobalRequest logic in post-read-request and
> header-parser phases, via RUN_FIRST.  since we moved user-defined
> PerlInitHandler logic to RUN_REALLY_FIRST, its possible that user-defined
> Perl handlers will run _before_ mod_perl gets the chance to insert its
> global logic.  I'm not sure if this is a bad thing, but it sounds bad.  so,
> I created a MODPERL_HOOK_REALLY_FIRST define (-20) to put the +GlobalRequest
> logic really, really first for those two phases.  please review.

very nice work, geoff! see some comments below.


> Index: src/modules/perl/mod_perl.h
> ===================================================================
> RCS file: /home/cvspublic/modperl-2.0/src/modules/perl/mod_perl.h,v
> retrieving revision 1.61
> diff -u -r1.61 mod_perl.h
> --- src/modules/perl/mod_perl.h	22 Sep 2003 17:43:41 -0000	1.61
> +++ src/modules/perl/mod_perl.h	27 Feb 2004 16:33:23 -0000
> @@ -108,4 +108,7 @@
>                                                  const char *,
>                                                  const char *);
>  
> +/* we need to hook a few internal things before APR_HOOK_REALLY_FIRST */
> +#define MODPERL_HOOK_REALLY_FIRST (-20)

The name is confusing, since APR_REALLY_FIRST is -10 and you kept the rest of 
the MODPERL_HOOK_ names matching . Call it REALLY_REALLY_FIRST? or anything 
else which is not _REALLY_FIRST?

>  #endif /*  MOD_PERL_H */
> Index: src/modules/perl/modperl_cmd.c
> ===================================================================
> RCS file: /home/cvspublic/modperl-2.0/src/modules/perl/modperl_cmd.c,v
> retrieving revision 1.57
> diff -u -r1.57 modperl_cmd.c
> --- src/modules/perl/modperl_cmd.c	14 Feb 2004 04:25:01 -0000	1.57
> +++ src/modules/perl/modperl_cmd.c	27 Feb 2004 16:33:23 -0000
> @@ -142,6 +142,62 @@
>      return NULL;
>  }
>  
> +MP_CMD_SRV_DECLARE(order)

please s/order/hook_order/ or similar, order on its own is too vague of a name.

> Index: src/modules/perl/modperl_config.c
> ===================================================================
> RCS file: /home/cvspublic/modperl-2.0/src/modules/perl/modperl_config.c,v
> retrieving revision 1.76
> diff -u -r1.76 modperl_config.c
> --- src/modules/perl/modperl_config.c	14 Feb 2004 01:38:05 -0000	1.76
> +++ src/modules/perl/modperl_config.c	27 Feb 2004 16:33:23 -0000
> @@ -179,6 +179,9 @@
>      scfg->gtop = modperl_gtop_new(p);
>  #endif        
>  
> +    /* no merge required - applies to the main server only */
> +    scfg->hook_order = apr_table_make(p, 2);

I think it's a waste. If it's used only by the main server and relevant only 
at the startup, just use some static variable and don't blow the size of scfg 
for vhosts.

>      /* must copy ap_server_argv0, because otherwise any read/write of
>       * $0 corrupts process' argv[0] (visible with 'ps -ef' on most
>       * unices). This is due to the logic of calculating PL_origalen in
> Index: src/modules/perl/modperl_types.h
> ===================================================================
> RCS file: /home/cvspublic/modperl-2.0/src/modules/perl/modperl_types.h,v
> retrieving revision 1.73
> diff -u -r1.73 modperl_types.h
> --- src/modules/perl/modperl_types.h	12 Feb 2004 02:05:28 -0000	1.73
> +++ src/modules/perl/modperl_types.h	27 Feb 2004 16:33:23 -0000
> @@ -137,6 +137,7 @@
>      modperl_options_t *flags;
>      apr_hash_t *modules;
>      server_rec *server;
> +    MpHV *hook_order;
>  } modperl_config_srv_t;

see above.

>  typedef struct {
> Index: src/modules/perl/modperl_util.c
> ===================================================================
> RCS file: /home/cvspublic/modperl-2.0/src/modules/perl/modperl_util.c,v
> retrieving revision 1.62
> diff -u -r1.62 modperl_util.c
> --- src/modules/perl/modperl_util.c	12 Feb 2004 02:05:28 -0000	1.62
> +++ src/modules/perl/modperl_util.c	27 Feb 2004 16:33:23 -0000
> @@ -843,3 +843,103 @@
>      /* copy the SV in case the pool goes out of scope before the perl scalar */
>      return newSVpv(ap_server_root_relative(p, fname), 0);
>  }
> +
> +/* from here down is support for dynamic hook ordering.  this is mostly
> + * stolen from mod_info.c, so see also the logic and descriptions there.
> + */

That's a bad idea to point to description in another module. What happens if 
it gets nuked/rewritten and it won't match the code below any longer. Just 
copy it over while it's correct.

The whole sorting code doesn't belong to modperl_util.c, IMHO. It's a one-time 
use code and should be static in where it is used, i.e. mod_perl.c

And finally, if there are already two modules who need to copy so much cruft 
to resort hooks, perhaps it should become an Apache API, so that you don't 
have to worry about win32 users and what not. This stuff is really not urgent 
to put in, IMHO. So if it gets added in 2.0.49 that's still cool with us, no?

__________________________________________________________________
Stas Bekman            JAm_pH ------> Just Another mod_perl Hacker
http://stason.org/     mod_perl Guide ---> http://perl.apache.org
mailto:stas@stason.org http://use.perl.org http://apacheweek.com
http://modperlbook.org http://apache.org   http://ticketmaster.com

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org


Re: dynamic request hook ordering (take 2)

Posted by Stas Bekman <st...@stason.org>.
Randy Kobes wrote:

>><IfModule mpm_winnt.c>
>>    ThreadsPerChild      50
>>    MaxRequestsPerChild  0
>></IfModule>
>>
>>The standard mp2 testsuite now passes all tests successful.
> 
> 
> I forgot to mention - I've also found lately that raising
> this limit is necessary.

Any idea why? 50 threads is not enough to run the tests, when with prefork 
only 2 workers are needed at most. Do we have some threads leak?

__________________________________________________________________
Stas Bekman            JAm_pH ------> Just Another mod_perl Hacker
http://stason.org/     mod_perl Guide ---> http://perl.apache.org
mailto:stas@stason.org http://use.perl.org http://apacheweek.com
http://modperlbook.org http://apache.org   http://ticketmaster.com

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org


Re: dynamic request hook ordering (take 2)

Posted by Geoffrey Young <ge...@modperlcookbook.org>.
> I just got hold of VC++ 6.0 and was waiting for 2.0.49 to be release before
> trying to compile everything again.  but now that it's out I'll give it all
> another whirl with the more recent version and see how it goes.
> 
> one of the things on my hit list is post-connect being called 4x instead of
> 2x like on unix, since that seems to be a major difference between the
> platforms.  but no sense guessing...

well, I think I'm about to give up.  I've determined that the problem isn't
in my logic but rather in some other interaction between httpd and mod_perl.
 the attached test case proves it to me - the hook order logic
(mod_hook_order.c) runs just fine as it's own post_config handler, but when
the same block is copied into mod_perl's post_config handler I see the
familiar invalid handle errors.

so, I'm probably going to drop it as I'm out of ideas.  thanks all for
taking the time to help me debug, though.

--Geoff

Re: dynamic request hook ordering (take 2)

Posted by Stas Bekman <st...@stason.org>.
Geoffrey Young wrote:
> 
>>>one of the things on my hit list is post-connect being called 4x
>>>instead of
>>>2x like on unix, since that seems to be a major difference between the
>>>platforms.  but no sense guessing...
>>
>>
>>Earlier Bill posted a trick how to avoid this situation (i.e.
>>controlling how many times a certain code is called).
>>
> 
> 
> yeah, that's what I was planning on implementing to see if it made a difference.

though i'd suggest to use the abstracted API that I've posted to httpd-dev. So 
we can just check the flag and next expose it to users so that they can 
control their own post-config and open-logs phases to make sure that they will 
run only once. That's what mp1 had for $Starting and $Restarting, but i'd 
suggest to revise that API and use a counter instead, since as we can see some 
mpms have it more than 2 times.

__________________________________________________________________
Stas Bekman            JAm_pH ------> Just Another mod_perl Hacker
http://stason.org/     mod_perl Guide ---> http://perl.apache.org
mailto:stas@stason.org http://use.perl.org http://apacheweek.com
http://modperlbook.org http://apache.org   http://ticketmaster.com

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org


Re: dynamic request hook ordering (take 2)

Posted by Geoffrey Young <ge...@modperlcookbook.org>.

>> one of the things on my hit list is post-connect being called 4x
>> instead of
>> 2x like on unix, since that seems to be a major difference between the
>> platforms.  but no sense guessing...
> 
> 
> Earlier Bill posted a trick how to avoid this situation (i.e.
> controlling how many times a certain code is called).
> 

yeah, that's what I was planning on implementing to see if it made a difference.

--Geoff


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org


Re: dynamic request hook ordering (take 2)

Posted by Stas Bekman <st...@stason.org>.
Geoffrey Young wrote:
>>Well, now that 2.0.49 is out I was finally able to try the latest Apache 
>>stuff with your hook ordering patch (I still don't know to build CVS 
>>Apache 2 on Win32), but unfortunately it still doesn't work.  The usual 
>>invalid handle error, same as before.
> 
> 
> thanks for trying :)
> 
> I just got hold of VC++ 6.0 and was waiting for 2.0.49 to be release before
> trying to compile everything again.  but now that it's out I'll give it all
> another whirl with the more recent version and see how it goes.
> 
> one of the things on my hit list is post-connect being called 4x instead of
> 2x like on unix, since that seems to be a major difference between the
> platforms.  but no sense guessing...

Earlier Bill posted a trick how to avoid this situation (i.e. controlling how 
many times a certain code is called).

-- 
__________________________________________________________________
Stas Bekman            JAm_pH ------> Just Another mod_perl Hacker
http://stason.org/     mod_perl Guide ---> http://perl.apache.org
mailto:stas@stason.org http://use.perl.org http://apacheweek.com
http://modperlbook.org http://apache.org   http://ticketmaster.com

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org


Re: dynamic request hook ordering (take 2)

Posted by Geoffrey Young <ge...@modperlcookbook.org>.
> Well, now that 2.0.49 is out I was finally able to try the latest Apache 
> stuff with your hook ordering patch (I still don't know to build CVS 
> Apache 2 on Win32), but unfortunately it still doesn't work.  The usual 
> invalid handle error, same as before.

thanks for trying :)

I just got hold of VC++ 6.0 and was waiting for 2.0.49 to be release before
trying to compile everything again.  but now that it's out I'll give it all
another whirl with the more recent version and see how it goes.

one of the things on my hit list is post-connect being called 4x instead of
2x like on unix, since that seems to be a major difference between the
platforms.  but no sense guessing...

--Geoff


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org


Re: dynamic request hook ordering (take 2)

Posted by Steve Hay <st...@uk.radan.com>.
Steve Hay wrote:

> Geoffrey Young wrote:
>
>> so cool, we've isolated the "I can't start problem" to something in
>> apr_hook_sort_all().
>>
>> so I guess the place to start looking is apr_hooks.c.  I'll take a 
>> look at
>> it some more and throw in some debugging statement to see if anything 
>> jumps
>> out at me.  one thing I've done already is to add some more details 
>> to the
>> core sort (attached).
>>
> I applied your hooks diff to the "hook_order-final2.patch" version and 
> tried to start up the server in one-process mode with "-D 
> ReallyLast".  I also turned on most MOD_PERL_TRACE flags too if that 
> helps any.  Attached is the error_log and console output.
>
> I noticed that your hooks diff was clearly produced against a later 
> version of apr_hooks.c than I have (2.0.48).  Presumably you're using 
> CVS Apache2?  I wonder if that makes any difference. 

Well, now that 2.0.49 is out I was finally able to try the latest Apache 
stuff with your hook ordering patch (I still don't know to build CVS 
Apache 2 on Win32), but unfortunately it still doesn't work.  The usual 
invalid handle error, same as before.

Just ruling out another possibility :(

- Steve



------------------------------------------------
Radan Computational Ltd.

The information contained in this message and any files transmitted with it are confidential and intended for the addressee(s) only.  If you have received this message in error or there are any problems, please notify the sender immediately.  The unauthorized use, disclosure, copying or alteration of this message is strictly forbidden.  Note that any views or opinions presented in this email are solely those of the author and do not necessarily represent those of Radan Computational Ltd.  The recipient(s) of this message should check it and any attached files for viruses: Radan Computational will accept no liability for any damage caused by any virus transmitted by this email.


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org


Re: dynamic request hook ordering (take 2)

Posted by Steve Hay <st...@uk.radan.com>.
Geoffrey Young wrote:

>>>the only thing I could suggest at this point is to comment out the
>>>apr_hook_sort_all() call in modperl_apache_resort_hooks() to see if there
>>>isn't something about that function (or calling it) that is mucking up the
>>>works.  what that would leave is the altering of the hook structure but (in
>>>theory) have no real effect since the hook order is the same as it was going in.
>>>
>>>      
>>>
>>That makes a difference:  The original test suite now gets as far as the 
>>second test (-DReallyLast)
>>    
>>
>
>  
>
>>There's nothing interesting in the error_log, and the test suite is 
>>aborted there.  I assume the failure is simply because what's being 
>>tested for hasn't actually been done this time.
>>    
>>
>
>right - the test should fail because the order hasn't actually changed here.
> but it ran.
>
>so cool, we've isolated the "I can't start problem" to something in
>apr_hook_sort_all().
>
>so I guess the place to start looking is apr_hooks.c.  I'll take a look at
>it some more and throw in some debugging statement to see if anything jumps
>out at me.  one thing I've done already is to add some more details to the
>core sort (attached).
>
I applied your hooks diff to the "hook_order-final2.patch" version and 
tried to start up the server in one-process mode with "-D ReallyLast".  
I also turned on most MOD_PERL_TRACE flags too if that helps any.  
Attached is the error_log and console output.

I noticed that your hooks diff was clearly produced against a later 
version of apr_hooks.c than I have (2.0.48).  Presumably you're using 
CVS Apache2?  I wonder if that makes any difference.

I tried to upgrade my Apache2 to the latest CVS, but couldn't.  I got 
all the latest httpd-2.0, apr, apr-iconv and apr-util code from 
cvs.apache.org, but none of it builds on Win32!  Those sources all 
correspond to the httpd-2.X.YY.tar.gz distributions that you guys use on 
UNIX, but those distributions don't build on Win32.  We have to use 
httpd-2.X.YY-win32-src.zip on Win32 instead.  The Win32 ZIP file 
contains extra *.mak and *.dep files needed to build stuff.  I tried 
copying the extra files from 2.0.48 into my CVS source tree, but I can't 
get the damn thing to build -- the makefiles all clearly need updating :(

Anyone know where to get the latest Win32 makefiles etc for Apache2 from?

- Steve


------------------------------------------------
Radan Computational Ltd.

The information contained in this message and any files transmitted with it are confidential and intended for the addressee(s) only.  If you have received this message in error or there are any problems, please notify the sender immediately.  The unauthorized use, disclosure, copying or alteration of this message is strictly forbidden.  Note that any views or opinions presented in this email are solely those of the author and do not necessarily represent those of Radan Computational Ltd.  The recipient(s) of this message should check it and any attached files for viruses: Radan Computational will accept no liability for any damage caused by any virus transmitted by this email.

Re: dynamic request hook ordering (take 2)

Posted by Geoffrey Young <ge...@modperlcookbook.org>.
>>ok.  I don't suppose that sleeping between startups makes a difference?
>>
> 
> Sleeping between what?  One test /on its own/ doesn't work!

yes, I realized after I sent it that you already found the problem has
nothing to do with stopping or starting.  oops :)

>>the only thing I could suggest at this point is to comment out the
>>apr_hook_sort_all() call in modperl_apache_resort_hooks() to see if there
>>isn't something about that function (or calling it) that is mucking up the
>>works.  what that would leave is the altering of the hook structure but (in
>>theory) have no real effect since the hook order is the same as it was going in.
>>
> 
> That makes a difference:  The original test suite now gets as far as the 
> second test (-DReallyLast)

> There's nothing interesting in the error_log, and the test suite is 
> aborted there.  I assume the failure is simply because what's being 
> tested for hasn't actually been done this time.

right - the test should fail because the order hasn't actually changed here.
 but it ran.

so cool, we've isolated the "I can't start problem" to something in
apr_hook_sort_all().

so I guess the place to start looking is apr_hooks.c.  I'll take a look at
it some more and throw in some debugging statement to see if anything jumps
out at me.  one thing I've done already is to add some more details to the
core sort (attached).  that gives me stuff like this:

[ from setup_prelinked_modules I think ]
Sorting translate_name: core.c (30)

[ from the main loop ]
Sorting translate_name: mod_perl.c (-10) mod_proxy.c (0) mod_rewrite.c (0)
mod_hook_order.c (0) mod_alias.c (10) mod_userdir.c (10) mod_vhost_alias.c
(10) core.c (30)

[ for mod_perl's post-config call ]
Sorting translate_name: mod_proxy.c (0) mod_rewrite.c (0) mod_hook_order.c
(0) mod_alias.c (10) mod_userdir.c (10) mod_vhost_alias.c (10) mod_perl.c
(30) core.c (30)

anyway, probably unimportant but I wanted to make sure that my new int
didn't get corrupt someplace.  ymmv :)

>>anyway, thanks so much for taking the time to play with this.  I hope it's
>>fun at least :)
>>
> 
> For the appropriate definition of "fun", yes ;)

:)

--Geoff

Re: dynamic request hook ordering (take 2)

Posted by Steve Hay <st...@uk.radan.com>.
Geoffrey Young wrote:

>>The ReallyFirst test runs OK on its own, though.
>>    
>>
>
>it seems I forgot to add a -DReallyFirst, so this is running the default as
>well.
>
Doh!  Have you added it now?  I've built & installed this new patch and 
I still find that -DReallyFirst works fine...

>
>  
>
>>    
>>
>>>finally, is there something screwy with my IO in t/TEST - if you take out
>>>all the open and print foo does the problem go away?  that was mostly for my
>>>debugging information anyway...
>>>
>>>      
>>>
>>Removing all the open/print/close stuff makes no difference.
>>    
>>
>
>ok.  I don't suppose that sleeping between startups makes a difference?
>
Sleeping between what?  One test /on its own/ doesn't work!

In fact, even this produces the usual error in the log and crashes the 
server:

C:\Temp\hook_order_test-mp2>C:/apache2/bin/Apache.exe -d 
C:/Temp/hook_order_test-mp2/t -f 
C:/Temp/hook_order_test-mp2/t/conf/httpd.conf -D APACHE2 -D 
PERL_USEITHREADS -D ReallyLast -X

(That's what I was running through the debugger.  I meant to say.)

>[...]
>  
>
>I spent the entire morning trolling through the winnt mpm and win32 API
>documentation and I'm stumped - I can't see the interaction between this
>code and what I'm doing at all.  furthermore, the two events are created far
>_after_ I am finished doing stuff, since ap_run_mpm is called after
>post-config, so it's not like I am erasing a previously initialized state.
>
>anyway, attached is yet another patch.  the main difference here is in the
>pools, so maybe that will have a positive effect :)  
>
Sadly not.  I still get the same failure as before.

>stas at one point
>suggested nulling the table when I'm done with it, which might also be an
>idea - IIRC, there is some config-parsing done on shutdown as well, so maybe
>something is getting stuck using the table created with a dead pool or
>something.
>
[I'll try your other (off-list) patch in a moment]

>
>the only thing I could suggest at this point is to comment out the
>apr_hook_sort_all() call in modperl_apache_resort_hooks() to see if there
>isn't something about that function (or calling it) that is mucking up the
>works.  what that would leave is the altering of the hook structure but (in
>theory) have no real effect since the hook order is the same as it was going in.
>
That makes a difference:  The original test suite now gets as far as the 
second test (-DReallyLast), which does this:

C:\apache2\bin\Apache.exe -d C:/Temp/hook_order_test-mp2/t -f 
C:/Temp/hook_order_test-mp2/t/conf/httpd.conf -D APACHE2 -D 
PERL_USEITHREADS -D ReallyLast
using Apache/2.0.48 (winnt MPM)

waiting 60 seconds for server to start: .
waiting 60 seconds for server to start: ok (waited 0 secs)
server localhost:8529 started
t\01first....skipped
        all skipped: no reason given
t\02last.....# Failed test 1 in t\02last.t at line 42
t\02last.....FAILED test 1
        Failed 1/1 tests, 0.00% okay
t\03mix......skipped
        all skipped: no reason given
Failed Test Stat Wstat Total Fail  Failed  List of Failed
-------------------------------------------------------------------------------
t\02last.t                 1    1 100.00%  1
2 tests skipped.

There's nothing interesting in the error_log, and the test suite is 
aborted there.  I assume the failure is simply because what's being 
tested for hasn't actually been done this time.

All the tests run OK (albeit with similar failures) in isolation.

>
>anyway, thanks so much for taking the time to play with this.  I hope it's
>fun at least :)
>
For the appropriate definition of "fun", yes ;)

- Steve



------------------------------------------------
Radan Computational Ltd.

The information contained in this message and any files transmitted with it are confidential and intended for the addressee(s) only.  If you have received this message in error or there are any problems, please notify the sender immediately.  The unauthorized use, disclosure, copying or alteration of this message is strictly forbidden.  Note that any views or opinions presented in this email are solely those of the author and do not necessarily represent those of Radan Computational Ltd.  The recipient(s) of this message should check it and any attached files for viruses: Radan Computational will accept no liability for any damage caused by any virus transmitted by this email.


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org


Re: dynamic request hook ordering (take 2)

Posted by Steve Hay <st...@uk.radan.com>.
Geoffrey Young wrote:

>Geoffrey Young wrote:
>  
>
>>stas at one point
>>suggested nulling the table when I'm done with it, which might also be an
>>idea
>>    
>>
>
>to that end, here is yet another patch to try.
>
Nope :(  Still the same results for me I'm afraid.

- Steve



------------------------------------------------
Radan Computational Ltd.

The information contained in this message and any files transmitted with it are confidential and intended for the addressee(s) only.  If you have received this message in error or there are any problems, please notify the sender immediately.  The unauthorized use, disclosure, copying or alteration of this message is strictly forbidden.  Note that any views or opinions presented in this email are solely those of the author and do not necessarily represent those of Radan Computational Ltd.  The recipient(s) of this message should check it and any attached files for viruses: Radan Computational will accept no liability for any damage caused by any virus transmitted by this email.


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org


Re: dynamic request hook ordering (take 2)

Posted by Geoffrey Young <ge...@modperlcookbook.org>.
>>I'm still unclear as to whether this is a fault of the new hook-ordering
>>implementation, or a symptom of the way I chose to setup the tests.  does it
>>behave the same way against pristine mod_perl CVS?
>>
> 
> How can I try that?!  If I run it against pristine mp2 CVS then it 
> complains "Invalid command 'PerlHookPostReadRequestHandler'" etc ;)

ah, yes.  duh :)

I guess what I was getting at, though, is the start/stop/start idea, but
your other work shows that's not an issue.

>>likewise, if you comment out everything in t/TEST save one test (any one
>>save the first, which doesn't excercise the new directives) does the
>>individual test pass?
>>
> 
> The first test alone obviously runs OK, and more importantly multiple 
> runs of the first test go OK:
> 
> Apache::TestRunPerl->new->run(@ARGV, '-defines', '');
> Apache::TestRunPerl->new->run(@ARGV, '-defines', '');

ok.

> 
> The ReallyLast, Last and Mix tests all fail when tried out 
> individually.  

rats.  it seems that the issue is clearly related to what I'm doing then.
it's almost like I suspected, that this feature would be adding a bit of
instability for little value.  but I/we are hip deep in it now, so it might
as well get fixed rather than abandoned ;)

> The ReallyFirst test runs OK on its own, though.

it seems I forgot to add a -DReallyFirst, so this is running the default as
well.

> 
> 
>>finally, is there something screwy with my IO in t/TEST - if you take out
>>all the open and print foo does the problem go away?  that was mostly for my
>>debugging information anyway...
>>
> 
> Removing all the open/print/close stuff makes no difference.

ok.  I don't suppose that sleeping between startups makes a difference?

> 
> (I'm testing this with "perl -Mblib t/TEST.PL" rather than "nmake test", 
> btw.)

sounds fine to me.

> 
> Still can't understand where that "Invalid handle" error comes from.  
> Here's what's on the call stack when that error comes out:
> 
> child_main(apr_pool_t * 0x6ff1d724) line 946
> ap_mpm_run(apr_pool_t * 0x0026abd8, apr_pool_t * 0x00848240, server_rec 
> * 0x0026ca70) line 1629
> main(int 4201858, const char * const * 0x0000000c) line 660 + 13 bytes
> APACHE! mainCRTStartup + 227 bytes
> KERNEL32! 77e814c7()
> 
> The (HANDLE *) child_events that is passed to WaitForMultipleObjects() 
> on line 935 of server/mpm/winnt/child.c is a pointer to an array of 
> object handles.  The first argument (here, 2) specifies the number of 
> object handles in that array.  Here's what the MSVC debugger shows for 
> child_events as it is passed to WaitForMultipleObjects:
> 
> child_events   0x0006fed4
> [0]            0x00000cb0
> [1]            0x00000000
> 
> The two events are exit_event and max_requests_per_child_event 
> respectively.  Is it significant that the latter is NULL and the former 
> is not?  Are they both supposed to be NULL or non-NULL?  Maybe the NULL 
> one is the "invalid handle", but why does it happen?  There is a comment 
> in the code about thread exits not being checked.  I don't know if 
> that's relevant or not?
> 
> I tried increasing ThreadsPerChild further, and fiddled with 
> MaxRequestsPerChild too, but to no avail.

I spent the entire morning trolling through the winnt mpm and win32 API
documentation and I'm stumped - I can't see the interaction between this
code and what I'm doing at all.  furthermore, the two events are created far
_after_ I am finished doing stuff, since ap_run_mpm is called after
post-config, so it's not like I am erasing a previously initialized state.

anyway, attached is yet another patch.  the main difference here is in the
pools, so maybe that will have a positive effect :)  stas at one point
suggested nulling the table when I'm done with it, which might also be an
idea - IIRC, there is some config-parsing done on shutdown as well, so maybe
something is getting stuck using the table created with a dead pool or
something.

the only thing I could suggest at this point is to comment out the
apr_hook_sort_all() call in modperl_apache_resort_hooks() to see if there
isn't something about that function (or calling it) that is mucking up the
works.  what that would leave is the altering of the hook structure but (in
theory) have no real effect since the hook order is the same as it was going in.

anyway, thanks so much for taking the time to play with this.  I hope it's
fun at least :)

--Geoff

Re: dynamic request hook ordering (take 2)

Posted by Steve Hay <st...@uk.radan.com>.
Geoffrey Young wrote:

>>I've now got the same problem as you, Randy -- it runs one set of tests 
>>OK (all successful), but then hangs trying to start the server the 
>>second time.
>>    
>>
>
>consistency is good :)
>
>I'm still unclear as to whether this is a fault of the new hook-ordering
>implementation, or a symptom of the way I chose to setup the tests.  does it
>behave the same way against pristine mod_perl CVS?
>
How can I try that?!  If I run it against pristine mp2 CVS then it 
complains "Invalid command 'PerlHookPostReadRequestHandler'" etc ;)

>
>likewise, if you comment out everything in t/TEST save one test (any one
>save the first, which doesn't excercise the new directives) does the
>individual test pass?
>
The first test alone obviously runs OK, and more importantly multiple 
runs of the first test go OK:

Apache::TestRunPerl->new->run(@ARGV, '-defines', '');
Apache::TestRunPerl->new->run(@ARGV, '-defines', '');

The ReallyLast, Last and Mix tests all fail when tried out 
individually.  The ReallyFirst test runs OK on its own, though.

>
>finally, is there something screwy with my IO in t/TEST - if you take out
>all the open and print foo does the problem go away?  that was mostly for my
>debugging information anyway...
>
Removing all the open/print/close stuff makes no difference.

(I'm testing this with "perl -Mblib t/TEST.PL" rather than "nmake test", 
btw.)

Still can't understand where that "Invalid handle" error comes from.  
Here's what's on the call stack when that error comes out:

child_main(apr_pool_t * 0x6ff1d724) line 946
ap_mpm_run(apr_pool_t * 0x0026abd8, apr_pool_t * 0x00848240, server_rec 
* 0x0026ca70) line 1629
main(int 4201858, const char * const * 0x0000000c) line 660 + 13 bytes
APACHE! mainCRTStartup + 227 bytes
KERNEL32! 77e814c7()

The (HANDLE *) child_events that is passed to WaitForMultipleObjects() 
on line 935 of server/mpm/winnt/child.c is a pointer to an array of 
object handles.  The first argument (here, 2) specifies the number of 
object handles in that array.  Here's what the MSVC debugger shows for 
child_events as it is passed to WaitForMultipleObjects:

child_events   0x0006fed4
[0]            0x00000cb0
[1]            0x00000000

The two events are exit_event and max_requests_per_child_event 
respectively.  Is it significant that the latter is NULL and the former 
is not?  Are they both supposed to be NULL or non-NULL?  Maybe the NULL 
one is the "invalid handle", but why does it happen?  There is a comment 
in the code about thread exits not being checked.  I don't know if 
that's relevant or not?

I tried increasing ThreadsPerChild further, and fiddled with 
MaxRequestsPerChild too, but to no avail.

- Steve



------------------------------------------------
Radan Computational Ltd.

The information contained in this message and any files transmitted with it are confidential and intended for the addressee(s) only.  If you have received this message in error or there are any problems, please notify the sender immediately.  The unauthorized use, disclosure, copying or alteration of this message is strictly forbidden.  Note that any views or opinions presented in this email are solely those of the author and do not necessarily represent those of Radan Computational Ltd.  The recipient(s) of this message should check it and any attached files for viruses: Radan Computational will accept no liability for any damage caused by any virus transmitted by this email.


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org


Re: dynamic request hook ordering (take 2)

Posted by Geoffrey Young <ge...@modperlcookbook.org>.
> I've now got the same problem as you, Randy -- it runs one set of tests 
> OK (all successful), but then hangs trying to start the server the 
> second time.

consistency is good :)

I'm still unclear as to whether this is a fault of the new hook-ordering
implementation, or a symptom of the way I chose to setup the tests.  does it
behave the same way against pristine mod_perl CVS?

likewise, if you comment out everything in t/TEST save one test (any one
save the first, which doesn't excercise the new directives) does the
individual test pass?

finally, is there something screwy with my IO in t/TEST - if you take out
all the open and print foo does the problem go away?  that was mostly for my
debugging information anyway...

--Geoff


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org


Re: dynamic request hook ordering (take 2)

Posted by Steve Hay <st...@uk.radan.com>.
Randy Kobes wrote:

>On Mon, 1 Mar 2004, Steve Hay wrote:
>  
>
>>What do I do with the hook_order_test-mp2.tar.gz tests?  Pardon my
>>ignorance, but I have no idea where to put these files or what to do
>>with them.
>>    
>>
>
>If I recall, you have the Win32 apxs installed? If so, all
>you need to do is unpack the tarball somewhere and run
>   perl Makefile.PL --apxs C:\Path\to\apxs.bat
>   nmake
>   nmake test
>This assumes that the patched mp2 has been installed.
>  
>
Thanks.  I'd left off the apxs bit.  Looks much better now ;)

I've now got the same problem as you, Randy -- it runs one set of tests 
OK (all successful), but then hangs trying to start the server the 
second time.  And like you, I have this in the error_log:

...
[Mon Mar 01 18:03:59 2004] [crit] (OS 6)The handle is invalid.  : Child 
3592: WAIT_FAILED -- shutting down server
...
[Mon Mar 01 18:03:59 2004] [warn] (OS 995)The I/O operation has been 
aborted because of either a thread exit or an application request.  : 
winnt_accept: Asynchronous AcceptEx failed.
[Mon Mar 01 18:03:59 2004] [info] Child 3592: Accept thread exiting.
[Mon Mar 01 18:04:00 2004] [notice] Child 3592: Released the start mutex
[Mon Mar 01 18:04:00 2004] [info] Child 3592: 50 threads blocked on the 
completion port
...

Well - at least there's two of us looking at it now...

- Steve



------------------------------------------------
Radan Computational Ltd.

The information contained in this message and any files transmitted with it are confidential and intended for the addressee(s) only.  If you have received this message in error or there are any problems, please notify the sender immediately.  The unauthorized use, disclosure, copying or alteration of this message is strictly forbidden.  Note that any views or opinions presented in this email are solely those of the author and do not necessarily represent those of Radan Computational Ltd.  The recipient(s) of this message should check it and any attached files for viruses: Radan Computational will accept no liability for any damage caused by any virus transmitted by this email.


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org


Re: dynamic request hook ordering (take 2)

Posted by Randy Kobes <ra...@theoryx5.uwinnipeg.ca>.
On Mon, 1 Mar 2004, Steve Hay wrote:

> Geoffrey Young wrote:
[ ... ]
> >ok, I'll hold off on committing then.  attached is the final patch I would
> >have put in this evening, fwiw.
> >
> >as I mentioned, I'd be interested to know if the test suite I had sent runs
> >against current pristine CVS - the individual tests would fail, obviously,
> >but if the server can't start/stop/start 5 times with the given -D switches
> >then at least I'd know it wasn't my fault (at least not currently :)
> >
> I've tried out the "final" patch with CVS mp2 (at about 15:30 01 Mar
> 2004 GMT) on WinXP / MSVC++ 6 with Apache 2.0.48 / Perl 5.8.3 and I
> found that when running "nmake test" (just the standard mp2 tests) the
> server now hangs after starting up.  The error_log contained:
>
> [Mon Mar 01 16:47:36 2004] [warn] Server ran out of threads to serve
> requests. Consider raising the ThreadsPerChild setting
>
> so I changed A-T/lib/Apache/TestConfig.pm in the obvious way:
>
> <IfModule mpm_winnt.c>
>     ThreadsPerChild      50
>     MaxRequestsPerChild  0
> </IfModule>
>
> The standard mp2 testsuite now passes all tests successful.

I forgot to mention - I've also found lately that raising
this limit is necessary.

> What do I do with the hook_order_test-mp2.tar.gz tests?  Pardon my
> ignorance, but I have no idea where to put these files or what to do
> with them.

If I recall, you have the Win32 apxs installed? If so, all
you need to do is unpack the tarball somewhere and run
   perl Makefile.PL --apxs C:\Path\to\apxs.bat
   nmake
   nmake test
This assumes that the patched mp2 has been installed.

-- 
best regards,
randy

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org


Re: dynamic request hook ordering (take 2)

Posted by Steve Hay <st...@uk.radan.com>.
Geoffrey Young wrote:

>>>or you could just comment out each
>>>different startup in TEST.PL (or just TEST if you don't feel like running
>>>Makefile.PL each time) and run each one at a time.
>>>      
>>>
>>I just did that - only two of the tests sequences run
>>individually - one where -defines is '' and the other when
>>it's 'ReallyFirst'. In fact, running the two together (ie,
>>when all the others are commented out) is fine - the server
>>starts, stops, and starts again fine, so that's not the
>>problem. 
>>    
>>
>
>blarg.
>
>ok, I'll hold off on committing then.  attached is the final patch I would
>have put in this evening, fwiw.
>
>as I mentioned, I'd be interested to know if the test suite I had sent runs
>against current pristine CVS - the individual tests would fail, obviously,
>but if the server can't start/stop/start 5 times with the given -D switches
>then at least I'd know it wasn't my fault (at least not currently :)
>
I've tried out the "final" patch with CVS mp2 (at about 15:30 01 Mar 
2004 GMT) on WinXP / MSVC++ 6 with Apache 2.0.48 / Perl 5.8.3 and I 
found that when running "nmake test" (just the standard mp2 tests) the 
server now hangs after starting up.  The error_log contained:

[Mon Mar 01 16:47:36 2004] [warn] Server ran out of threads to serve 
requests. Consider raising the ThreadsPerChild setting

so I changed A-T/lib/Apache/TestConfig.pm in the obvious way:

<IfModule mpm_winnt.c>
    ThreadsPerChild      50
    MaxRequestsPerChild  0
</IfModule>

The standard mp2 testsuite now passes all tests successful.

What do I do with the hook_order_test-mp2.tar.gz tests?  Pardon my 
ignorance, but I have no idea where to put these files or what to do 
with them.

- Steve



------------------------------------------------
Radan Computational Ltd.

The information contained in this message and any files transmitted with it are confidential and intended for the addressee(s) only.  If you have received this message in error or there are any problems, please notify the sender immediately.  The unauthorized use, disclosure, copying or alteration of this message is strictly forbidden.  Note that any views or opinions presented in this email are solely those of the author and do not necessarily represent those of Radan Computational Ltd.  The recipient(s) of this message should check it and any attached files for viruses: Radan Computational will accept no liability for any damage caused by any virus transmitted by this email.


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org


Re: dynamic request hook ordering (take 2)

Posted by Stas Bekman <st...@stason.org>.
Geoffrey Young wrote:

>>but you need to pass pool... so may be not... just another idea to make
>>mod_perl.c less cluttered.
> 
> 
> passing the pool isn't that big a deal, or I could just use
> modperl_global_get_pconf(), in which case I'll also need to use pconf for
> the logic in modperl_cmd then (instead of parms->pool) so that the table and
> data have the same lifetime.  using the global pool shouldn't be a big deal,
> since it's not a lot of data (and probably won't be any for 99% of the cases
> anyway).

Thinking forward I think you want to pass it explicitly from cmd. So if we do 
that temp pool/sub-pool solution we won't need to remember to change a lot of 
code and hunt for those global pool uses.

>>did you have a chance to look at the temp pool? if it does what I think
>>it does (i.e. get destroyed at the end of config) then you could use it
>>instead.
> 
> 
> no, I didn't trace the origins of those other two pools.  but as per the
> above assertion, the pools should match, right?  in which case it would be
> difficult to get into the modperl_cmd.c logic.

right, see above.

__________________________________________________________________
Stas Bekman            JAm_pH ------> Just Another mod_perl Hacker
http://stason.org/     mod_perl Guide ---> http://perl.apache.org
mailto:stas@stason.org http://use.perl.org http://apacheweek.com
http://modperlbook.org http://apache.org   http://ticketmaster.com

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org


Re: dynamic request hook ordering (take 2)

Posted by Geoffrey Young <ge...@modperlcookbook.org>.
> Actually, you don't even need modperl_apache_init_hook_order(), just
> move that init code into: modperl_apache_get_hook_order();
> 
> void modperl_apache_get_hook_order(apr_pool_t *p) {
>     if (!hook_order) {
>         hook_order = apr_table_make(p, 0);
>     }
>     return hook_order;
> }

I had suggested that before:

>> yeah, ok.  so I can keep the init in pre-config?  I guess if I use the
>> pconf
>> it doesn't really matter - I can have the accessor init using pconf if
>> the
>> table is NULL.  and I was doubting the lifetime of parms->pool anyway :)
>
>
> If we implement the sub-pool for temp uses, then it'll have to be in
> pre-config, so keep it there.

> 
> but you need to pass pool... so may be not... just another idea to make
> mod_perl.c less cluttered.

passing the pool isn't that big a deal, or I could just use
modperl_global_get_pconf(), in which case I'll also need to use pconf for
the logic in modperl_cmd then (instead of parms->pool) so that the table and
data have the same lifetime.  using the global pool shouldn't be a big deal,
since it's not a lot of data (and probably won't be any for 99% of the cases
anyway).

> 
> did you have a chance to look at the temp pool? if it does what I think
> it does (i.e. get destroyed at the end of config) then you could use it
> instead.

no, I didn't trace the origins of those other two pools.  but as per the
above assertion, the pools should match, right?  in which case it would be
difficult to get into the modperl_cmd.c logic.

--Geoff


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org


Re: dynamic request hook ordering (take 2)

Posted by Stas Bekman <st...@stason.org>.
Geoffrey Young wrote:
> 
>>It should be:
>>
>>    modperl_apache_init_hooks(p);
>>
> 
> 
>>void modperl_apache_init_hook_order(apr_pool_t p) {
>>    hook_order = apr_table_make(p, 0);
>>}
> 
> 
> ok, I'll do that.  I was trying to solve a very strange problem that I
> couldn't figure out (but would probably obvious to someone else, naturally)
>  - I was calling my get_hook_order() function from pre-init and initializing
> it, but for some reason the init wasn't taking.
> 
> but I like your solution much, much better.  thanks.

Actually, you don't even need modperl_apache_init_hook_order(), just move that 
init code into: modperl_apache_get_hook_order();

void modperl_apache_get_hook_order(apr_pool_t *p) {
     if (!hook_order) {
         hook_order = apr_table_make(p, 0);
     }
     return hook_order;
}

but you need to pass pool... so may be not... just another idea to make 
mod_perl.c less cluttered.

did you have a chance to look at the temp pool? if it does what I think it 
does (i.e. get destroyed at the end of config) then you could use it instead.

__________________________________________________________________
Stas Bekman            JAm_pH ------> Just Another mod_perl Hacker
http://stason.org/     mod_perl Guide ---> http://perl.apache.org
mailto:stas@stason.org http://use.perl.org http://apacheweek.com
http://modperlbook.org http://apache.org   http://ticketmaster.com

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org


Re: dynamic request hook ordering (take 2)

Posted by Geoffrey Young <ge...@modperlcookbook.org>.

> It should be:
> 
>     modperl_apache_init_hooks(p);
> 

> void modperl_apache_init_hook_order(apr_pool_t p) {
>     hook_order = apr_table_make(p, 0);
> }

ok, I'll do that.  I was trying to solve a very strange problem that I
couldn't figure out (but would probably obvious to someone else, naturally)
 - I was calling my get_hook_order() function from pre-init and initializing
it, but for some reason the init wasn't taking.

but I like your solution much, much better.  thanks.

--Geoff


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org


Re: dynamic request hook ordering (take 2)

Posted by Stas Bekman <st...@stason.org>.
Geoffrey Young wrote:

> Index: src/modules/perl/mod_perl.c
> ===================================================================
> RCS file: /home/cvs/modperl-2.0/src/modules/perl/mod_perl.c,v
> retrieving revision 1.210
> diff -u -r1.210 mod_perl.c
> --- src/modules/perl/mod_perl.c	1 Mar 2004 04:24:00 -0000	1.210
> +++ src/modules/perl/mod_perl.c	1 Mar 2004 04:57:56 -0000
> @@ -543,6 +543,11 @@
>  {
>      /* we can't have PerlPreConfigHandler without first configuring mod_perl */
>  
> +    /* PerlHook*Handler support */
> +    apr_table_t *hook_order = apr_table_make(p, 0);

why the duplicate? You already have hook_order static var in modperl_apache.c

It should be:

     modperl_apache_init_hooks(p);

with the above code moved to modperl_apache.c.
(see below).

> +    modperl_apache_set_hook_order(hook_order);

no need for this function at all.

>      /* perl 5.8.1+ */
>      modperl_hash_seed_init(p);
>  
> @@ -573,6 +578,9 @@
>      modperl_trace_logfile_set(s->error_log);
>  #endif
>      
> +    /* fixup the placement of user-defined Perl*Handlers in the hook order */
> +    modperl_apache_resort_hooks();

yup, just like that one. two calls from mod_perl.c but none really gets/passes 
hook_order.

> --- /dev/null	2003-01-30 05:24:37.000000000 -0500
> +++ src/modules/perl/modperl_apache.c	2004-02-29 23:42:16.000000000 -0500
> @@ -0,0 +1,115 @@
> +#include "mod_perl.h"
> +
> +/* PerlHook*Handler support */
> +static apr_table_t *hook_order;
> +
> +apr_table_t *modperl_apache_get_hook_order() {
> +    return hook_order;
> +}

void modperl_apache_init_hook_order(apr_pool_t p) {
     hook_order = apr_table_make(p, 0);
}

> +void modperl_apache_set_hook_order(apr_table_t *set_order) {
> +    hook_order = set_order;
> +}

nuke that one.
__________________________________________________________________
Stas Bekman            JAm_pH ------> Just Another mod_perl Hacker
http://stason.org/     mod_perl Guide ---> http://perl.apache.org
mailto:stas@stason.org http://use.perl.org http://apacheweek.com
http://modperlbook.org http://apache.org   http://ticketmaster.com

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org


Re: dynamic request hook ordering (take 2)

Posted by Geoffrey Young <ge...@modperlcookbook.org>.
>> or you could just comment out each
>>different startup in TEST.PL (or just TEST if you don't feel like running
>>Makefile.PL each time) and run each one at a time.
> 
> 
> I just did that - only two of the tests sequences run
> individually - one where -defines is '' and the other when
> it's 'ReallyFirst'. In fact, running the two together (ie,
> when all the others are commented out) is fine - the server
> starts, stops, and starts again fine, so that's not the
> problem. 

blarg.

ok, I'll hold off on committing then.  attached is the final patch I would
have put in this evening, fwiw.

as I mentioned, I'd be interested to know if the test suite I had sent runs
against current pristine CVS - the individual tests would fail, obviously,
but if the server can't start/stop/start 5 times with the given -D switches
then at least I'd know it wasn't my fault (at least not currently :)

anyway, thanks for tracking the issues down.  talk to you soon.

--Geoff

Re: dynamic request hook ordering (take 2)

Posted by Randy Kobes <ra...@theoryx5.uwinnipeg.ca>.
On Sun, 29 Feb 2004, Geoffrey Young wrote:

>
> > Hi Geoff,
>
> hey randy.  hope you had a nice vacation :)

Thanks :) Actually, it was a bunch of meetings for deciding
on some grants, which was interesting, but stressful ...

> >    However, when running the tests, what happens is that the
> > tests are first run, with 01first passing and 02last and
> > 03mix being skipped. Then the server shuts down, and is
> > restarted with -DReallyLast,
>
> since these are startup-only directives, that's the only way I could think
> to do it - shutdown and restart the server a few times, each time with a
> different config.
>
> > but it times out (I raised the
> > timeout to 240 seconds, but this didn't help).
>
> argh.  are you using the latest CVS, with the new "-D [space] DEFINE" stuff?
>  steve mentioned a hanging apache.exe, maybe that's conflicting with the
> restart?

I don't think that's the problem - the one I'm using doesn't
have a space after the -D; from what I understand, this was
more a mp1 problem.

> > I've attached
> > the error_log - does that help in seeing what goes wrong?
>
>
> well, win32 is a mystery to me.  maybe the A-T shutdown logic isn't as nice
> as we would like.
>
> I suppose you can add a few 'sleep N' calls to TEST.PL to see if giving it
> some extra time to shutdown helps.  or you could just comment out each
> different startup in TEST.PL (or just TEST if you don't feel like running
> Makefile.PL each time) and run each one at a time.

I just did that - only two of the tests sequences run
individually - one where -defines is '' and the other when
it's 'ReallyFirst'. In fact, running the two together (ie,
when all the others are commented out) is fine - the server
starts, stops, and starts again fine, so that's not the
problem. The failures all lead to

[Sun Feb 29 22:11:42 2004] [crit] (OS 6)The handle is
invalid.  : Child 3168: WAIT_FAILED -- shutting down server
[Sun Feb 29 22:11:42 2004] [debug] child.c(695): Child 3168:
Worker thread 19 starting.
[Sun Feb 29 22:11:42 2004] [warn] (OS 995)The I/O operation
has been aborted because of either a thread exit or an
application request.  : winnt_accept: Asynchronous AcceptEx
failed.

occurring in the error log.

I'll try to track down where the invalid handle is coming
from ....

-- 
best regards,
randy

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org


Re: dynamic request hook ordering (take 2)

Posted by Geoffrey Young <ge...@modperlcookbook.org>.
> Hi Geoff,

hey randy.  hope you had a nice vacation :)

>     I've tried this out on Win32 - nice work! Just a small
> thing - in src/modules/perl/modperl_apache.c, there's a
> declaration of 'int j' in modperl_apache_resort_hooks() that
> VC++ didn't like occurring so late - moving it up near the
> beginning where 'int i' is declared fixes that.

ok, I'll fix that.  I am running my final test now before committing.

>    However, when running the tests, what happens is that the
> tests are first run, with 01first passing and 02last and
> 03mix being skipped. Then the server shuts down, and is
> restarted with -DReallyLast, 

since these are startup-only directives, that's the only way I could think
to do it - shutdown and restart the server a few times, each time with a
different config.

> but it times out (I raised the
> timeout to 240 seconds, but this didn't help).

argh.  are you using the latest CVS, with the new "-D [space] DEFINE" stuff?
 steve mentioned a hanging apache.exe, maybe that's conflicting with the
restart?

> I've attached
> the error_log - does that help in seeing what goes wrong?

> [Sun Feb 29 22:11:42 2004] [crit] (OS 6)The handle is invalid.  : Child 3168: WAIT_FAILED -- shutting down server
> [Sun Feb 29 22:11:42 2004] [debug] child.c(695): Child 3168: Worker thread 19 starting.
> [Sun Feb 29 22:11:42 2004] [warn] (OS 995)The I/O operation has been aborted because of either a thread exit or an application request.  : winnt_accept: Asynchronous AcceptEx failed.

well, win32 is a mystery to me.  maybe the A-T shutdown logic isn't as nice
as we would like.

I suppose you can add a few 'sleep N' calls to TEST.PL to see if giving it
some extra time to shutdown helps.  or you could just comment out each
different startup in TEST.PL (or just TEST if you don't feel like running
Makefile.PL each time) and run each one at a time.

just to make sure it's the A-T logic and not the new patch, I'd be curious
if running the same test again from TEST.PL gives the same result without
patched sources. that is, without the test ordering foo added to the
mod_perl sources, can you

  Apache::TestRunPerl->new->run(@ARGV, '-defines', '');
  Apache::TestRunPerl->new->run(@ARGV, '-defines', '');

from t/TEST and have it work?

sorry to make you go through all this, though, especially when you're
testing for my benefit :)  but thanks a ton for taking the time.

--Geoff


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org


Re: dynamic request hook ordering (take 2)

Posted by Randy Kobes <ra...@theoryx5.uwinnipeg.ca>.
On Sun, 29 Feb 2004, Geoffrey Young wrote:

> here's another pass that incorporates the suggestions thus
> far.  comments on the global foo appreciated.  the tests I
> posted before are still valid at this point.
>
> --Geoff

Hi Geoff,
    I've tried this out on Win32 - nice work! Just a small
thing - in src/modules/perl/modperl_apache.c, there's a
declaration of 'int j' in modperl_apache_resort_hooks() that
VC++ didn't like occurring so late - moving it up near the
beginning where 'int i' is declared fixes that.
   However, when running the tests, what happens is that the
tests are first run, with 01first passing and 02last and
03mix being skipped. Then the server shuts down, and is
restarted with -DReallyLast, but it times out (I raised the
timeout to 240 seconds, but this didn't help). I've attached
the error_log - does that help in seeing what goes wrong?

-- 
best regards,
randy

Re: dynamic request hook ordering (take 2)

Posted by Stas Bekman <st...@stason.org>.
Geoffrey Young wrote:
> 
> Stas Bekman wrote:
> 
>>Geoffrey Young wrote:
>>
>>
>>>>Much better, but still why having any data at all in mod_perl.c? Have
>>>>the static variable in modperl_apache.c and provide an accessor to do:
>>>>
>>>>hook_order = apr_table_make(p, 0);
>>>>
>>>>from modperl_apache.c. Now you are all set.
>>>
>>>
>>>
>>>well, I can't see where to draw the pool from or how else to inialize it.
>>>that is, unless I just make the declaration
>>>
>>>static *hook_order = apr_table_make(modperl_global_get_pconf(), 0);
>>
>>
>>You could do that. But I was merely suggesting to have a function
>>defined in modperl_apache.c that you will call from mod_perl.c, instead
>>of accessing the raw variable.
> 
> 
> yeah, ok.  so I can keep the init in pre-config?  I guess if I use the pconf
> it doesn't really matter - I can have the accessor init using pconf if the
> table is NULL.  and I was doubting the lifetime of parms->pool anyway :)

If we implement the sub-pool for temp uses, then it'll have to be in 
pre-config, so keep it there.

>>I'm also thinking that we have this and a few other things that are used
>>only at the server startup and are no longer needed at run time. So may
>>be we should really create a sub-pool and destroy it when we are done
>>with it? e.g. create it at the preconfig phase and destroy it at the
>>postconfig phase? So we will save a bit of a memory.
> 
> 
> yeah, that might be a good idea.

BTW, what the temp_pool is for? Isn't that exactly the kind of pool that we 
want to use for config only things, that can be nuked when we are done? Does 
it get destroyed after the startup?

>>But that of course can be done separately, after you roll this change in.
> 
> 
> ok, cool.  I'll give the logic another once over and commit it tomorrow.

So you'd be the first one to commit in Spring ;) Unless gozer beats you with 
his static modperl build ;)

> I really, really appreciate the feedback and pointers.  thanks.

;)

__________________________________________________________________
Stas Bekman            JAm_pH ------> Just Another mod_perl Hacker
http://stason.org/     mod_perl Guide ---> http://perl.apache.org
mailto:stas@stason.org http://use.perl.org http://apacheweek.com
http://modperlbook.org http://apache.org   http://ticketmaster.com

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org


Re: dynamic request hook ordering (take 2)

Posted by Geoffrey Young <ge...@modperlcookbook.org>.

Stas Bekman wrote:
> Geoffrey Young wrote:
> 
>>> Much better, but still why having any data at all in mod_perl.c? Have
>>> the static variable in modperl_apache.c and provide an accessor to do:
>>>
>>> hook_order = apr_table_make(p, 0);
>>>
>>> from modperl_apache.c. Now you are all set.
>>
>>
>>
>> well, I can't see where to draw the pool from or how else to inialize it.
>> that is, unless I just make the declaration
>>
>> static *hook_order = apr_table_make(modperl_global_get_pconf(), 0);
> 
> 
> You could do that. But I was merely suggesting to have a function
> defined in modperl_apache.c that you will call from mod_perl.c, instead
> of accessing the raw variable.

yeah, ok.  so I can keep the init in pre-config?  I guess if I use the pconf
it doesn't really matter - I can have the accessor init using pconf if the
table is NULL.  and I was doubting the lifetime of parms->pool anyway :)

> 
> I'm also thinking that we have this and a few other things that are used
> only at the server startup and are no longer needed at run time. So may
> be we should really create a sub-pool and destroy it when we are done
> with it? e.g. create it at the preconfig phase and destroy it at the
> postconfig phase? So we will save a bit of a memory.

yeah, that might be a good idea.

> 
> But that of course can be done separately, after you roll this change in.

ok, cool.  I'll give the logic another once over and commit it tomorrow.

I really, really appreciate the feedback and pointers.  thanks.

--Geoff


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org


Re: dynamic request hook ordering (take 2)

Posted by Stas Bekman <st...@stason.org>.
Geoffrey Young wrote:
>>Much better, but still why having any data at all in mod_perl.c? Have
>>the static variable in modperl_apache.c and provide an accessor to do:
>>
>>hook_order = apr_table_make(p, 0);
>>
>>from modperl_apache.c. Now you are all set.
> 
> 
> well, I can't see where to draw the pool from or how else to inialize it.
> that is, unless I just make the declaration
> 
> static *hook_order = apr_table_make(modperl_global_get_pconf(), 0);

You could do that. But I was merely suggesting to have a function defined in 
modperl_apache.c that you will call from mod_perl.c, instead of accessing the 
raw variable.

I'm also thinking that we have this and a few other things that are used only 
at the server startup and are no longer needed at run time. So may be we 
should really create a sub-pool and destroy it when we are done with it? e.g. 
create it at the preconfig phase and destroy it at the postconfig phase? So we 
will save a bit of a memory.

But that of course can be done separately, after you roll this change in.

__________________________________________________________________
Stas Bekman            JAm_pH ------> Just Another mod_perl Hacker
http://stason.org/     mod_perl Guide ---> http://perl.apache.org
mailto:stas@stason.org http://use.perl.org http://apacheweek.com
http://modperlbook.org http://apache.org   http://ticketmaster.com

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org


Re: dynamic request hook ordering (take 2)

Posted by Geoffrey Young <ge...@modperlcookbook.org>.
> Much better, but still why having any data at all in mod_perl.c? Have
> the static variable in modperl_apache.c and provide an accessor to do:
> 
> hook_order = apr_table_make(p, 0);
> 
> from modperl_apache.c. Now you are all set.

well, I can't see where to draw the pool from or how else to inialize it.
that is, unless I just make the declaration

static *hook_order = apr_table_make(modperl_global_get_pconf(), 0);

> 
> Also, this:
> 
>> +/* PerlHook*Handler support */
>> +apr_table_t *hook_order;
> 
> 
> needs to be 'static'. Otherwise that symbol ends up in the final
> library. (again, please refer to Ulrich Drepper's paper: How to Write
> Shared Libraries http://people.redhat.com/drepper/ukuug2002slides.pdf)

k, thanks.

--Geoff


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org


Re: dynamic request hook ordering (take 2)

Posted by Stas Bekman <st...@stason.org>.
Geoffrey Young wrote:
> here's another pass that incorporates the suggestions thus far.  comments on
> the global foo appreciated.  the tests I posted before are still valid at
> this point.

Much better, but still why having any data at all in mod_perl.c? Have the 
static variable in modperl_apache.c and provide an accessor to do:

hook_order = apr_table_make(p, 0);

from modperl_apache.c. Now you are all set.

Also, this:

> +/* PerlHook*Handler support */
> +apr_table_t *hook_order;

needs to be 'static'. Otherwise that symbol ends up in the final library. 
(again, please refer to Ulrich Drepper's paper: How to Write Shared Libraries 
http://people.redhat.com/drepper/ukuug2002slides.pdf)

__________________________________________________________________
Stas Bekman            JAm_pH ------> Just Another mod_perl Hacker
http://stason.org/     mod_perl Guide ---> http://perl.apache.org
mailto:stas@stason.org http://use.perl.org http://apacheweek.com
http://modperlbook.org http://apache.org   http://ticketmaster.com

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org


Re: dynamic request hook ordering (take 2)

Posted by Geoffrey Young <ge...@modperlcookbook.org>.
here's another pass that incorporates the suggestions thus far.  comments on
the global foo appreciated.  the tests I posted before are still valid at
this point.

--Geoff

Re: dynamic request hook ordering (Win32 testers needed :)

Posted by Geoffrey Young <ge...@modperlcookbook.org>.
> This is exactly what I was talking about. Making it configurable at
> startup time. 

:)

actually, once I got it figured out, it's probably possible to support
per-dir hook ordering as well, but I don't think it's valuable enough to
warrant additonal effort.  but it's possible ;)

> I don't know about having one directive per handler, why
> not something like :
> 
> PerlHookOrder trans last
> 
> I don't mind the multiples directive much, and it's not like we are
> facing new directives all the time, but it just feels cleaner to me that
> way.

yeah, I was debating on that too.  I suppose it could be like that, or maybe

  PerlHookOrder PerlTransHandler Last

in either case, it would be fewer hooks but a larger validation routine.


>>  2 - on a parallel with this specific feature, I noticed that mod_perl
>>hooks PerlOptions +GlobalRequest logic in post-read-request and
>>header-parser phases, via RUN_FIRST.  since we moved user-defined
>>PerlInitHandler logic to RUN_REALLY_FIRST, its possible that user-defined
>>Perl handlers will run _before_ mod_perl gets the chance to insert its
>>global logic.  I'm not sure if this is a bad thing, but it sounds bad.
> 
> 
> Sure _sounds_ bad AFAIK

yeah, that's why I bumped the internal logic really, really first - a gut
feeling.


> Gota love those first, really first, our really first, our really really
> first, etc ;-)

:)

> 
> All in all, this looks like good works, see few comments below.

cool, thanks.

> Does this ordering information need to be in the server-configuration
> request since it's really a global?

yeah, stas pointed that out too.  I'll fix it on the next pass.

thanks.

--Geoff


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org


Re: dynamic request hook ordering (Win32 testers needed :)

Posted by "Philippe M. Chiasson" <go...@cpan.org>.
On Fri, 2004-02-27 at 09:17, Geoffrey Young wrote:
> hi all
> 
> ok, I spent some time and was able to get configuration-based dynamic hook
> ordering working.

Great!

> the attached patch adds a PerlHook*Handler directive for each request phase
> (minus the PerlResponseHandler).  so, you would have a configuration like this
> 
>   PerlHookTransHandler Last
> 
> valid values correspond to APR_HOOK* values: ReallyFirst, First, Middle,
> Last, ReallyLast.
> 
> the directives (and effects) are global, so they are not allowed in vhosts -
> you get one shot, just like you did with ClearModuleList/AddModule in 1.3.

This is exactly what I was talking about. Making it configurable at
startup time. I don't know about having one directive per handler, why
not something like :

PerlHookOrder trans last

I don't mind the multiples directive much, and it's not like we are
facing new directives all the time, but it just feels cleaner to me that
way.

> two things of note:
> 
>   1 - I really need someone on Win32 to give this a whirl.  there's some
> code in there (swiped from mod_info) that is Win32 specific so it needs to
> be excercised.  running both the mod_perl test suite as well as the attached
> tests would be greatly appreciated.

Sorry, no luck there from me, but on my non-win32 system:

All tests successful, 3 tests skipped.
Files=174, Tests=1081, 111 wallclock secs

>   2 - on a parallel with this specific feature, I noticed that mod_perl
> hooks PerlOptions +GlobalRequest logic in post-read-request and
> header-parser phases, via RUN_FIRST.  since we moved user-defined
> PerlInitHandler logic to RUN_REALLY_FIRST, its possible that user-defined
> Perl handlers will run _before_ mod_perl gets the chance to insert its
> global logic.  I'm not sure if this is a bad thing, but it sounds bad.

Sure _sounds_ bad AFAIK

>   so,
> I created a MODPERL_HOOK_REALLY_FIRST define (-20) to put the +GlobalRequest
> logic really, really first for those two phases.  please review.

Gota love those first, really first, our really first, our really really
first, etc ;-)

All in all, this looks like good works, see few comments below.

> --Geoff
> 
> ______________________________________________________________________
> Index: src/modules/perl/mod_perl.c
> ===================================================================
> RCS file: /home/cvspublic/modperl-2.0/src/modules/perl/mod_perl.c,v
> retrieving revision 1.208
> diff -u -r1.208 mod_perl.c
> --- src/modules/perl/mod_perl.c	13 Feb 2004 14:58:05 -0000	1.208
> +++ src/modules/perl/mod_perl.c	27 Feb 2004 16:33:23 -0000
> @@ -573,6 +573,9 @@
>      modperl_trace_logfile_set(s->error_log);
>  #endif
>      
> +    /* fixup the placement of mod_perl in the hook order */
> +    modperl_util_resort_hooks(scfg);
> +
>      ap_add_version_component(pconf, MP_VERSION_STRING);
>      ap_add_version_component(pconf,
>                               Perl_form(aTHX_ "Perl/v%vd", PL_patchlevel));
> @@ -713,11 +716,15 @@
>      ap_hook_create_request(modperl_hook_create_request,
>                             NULL, NULL, APR_HOOK_MIDDLE);
>  
> +    /* both of these hooks need to run really, really first.
> +     * otherwise, the global request_rec will be set up _after_ some
> +     * Perl handlers run.
> +     */
>      ap_hook_post_read_request(modperl_hook_post_read_request,
> -                              NULL, NULL, APR_HOOK_FIRST);
> +                              NULL, NULL, MODPERL_HOOK_REALLY_FIRST);
>  
>      ap_hook_header_parser(modperl_hook_header_parser,
> -                          NULL, NULL, APR_HOOK_FIRST);
> +                          NULL, NULL, MODPERL_HOOK_REALLY_FIRST);
>  
>      ap_hook_child_init(modperl_hook_child_init,
>                         NULL, NULL, APR_HOOK_FIRST);
> @@ -778,6 +785,26 @@
>      MP_CMD_SRV_FLAG("PerlWarn", warn,
>                      "Turn on -w switch"),
>  #endif
> +    MP_CMD_SRV_TAKE1("PerlHookPostReadRequestHandler", order,
> +                     "hook order for PerlPostReadRequestHandler"),
> +    MP_CMD_SRV_TAKE1("PerlHookTransHandler", order,
> +                     "hook order for PerlTransHandler"),
> +    MP_CMD_SRV_TAKE1("PerlHookMapToStorageHandler", order,
> +                     "hook order for PerlMapToStorageHandler"),
> +    MP_CMD_SRV_TAKE1("PerlHookHeaderParserHandler", order,
> +                     "hook order for PerlHeaderParserHandler"),
> +    MP_CMD_SRV_TAKE1("PerlHookAccessHandler", order,
> +                     "hook order for PerlAccessHandler"),
> +    MP_CMD_SRV_TAKE1("PerlHookAuthenHandler", order,
> +                     "hook order for PerlAuthenHandler"),
> +    MP_CMD_SRV_TAKE1("PerlHookAuthzHandler", order,
> +                     "hook order for PerlAuthzHandler"),
> +    MP_CMD_SRV_TAKE1("PerlHookTypeHandler", order,
> +                     "hook order for PerlTypeHandler"),
> +    MP_CMD_SRV_TAKE1("PerlHookFixupHandler", order,
> +                     "hook order for PerlFixupHandler"),
> +    MP_CMD_SRV_TAKE1("PerlHookLogHandler", order,
> +                     "hook order for PerlLogHandler"),
>      MP_CMD_ENTRIES,
>      { NULL }, 
>  }; 

Here, see my comment above about possibly using only one ordering
directive.

> Index: src/modules/perl/mod_perl.h
> ===================================================================
> RCS file: /home/cvspublic/modperl-2.0/src/modules/perl/mod_perl.h,v
> retrieving revision 1.61
> diff -u -r1.61 mod_perl.h
> --- src/modules/perl/mod_perl.h	22 Sep 2003 17:43:41 -0000	1.61
> +++ src/modules/perl/mod_perl.h	27 Feb 2004 16:33:23 -0000
> @@ -108,4 +108,7 @@
>                                                  const char *,
>                                                  const char *);
>  
> +/* we need to hook a few internal things before APR_HOOK_REALLY_FIRST */
> +#define MODPERL_HOOK_REALLY_FIRST (-20)
> +
>  #endif /*  MOD_PERL_H */
> Index: src/modules/perl/modperl_cmd.c
> ===================================================================
> RCS file: /home/cvspublic/modperl-2.0/src/modules/perl/modperl_cmd.c,v
> retrieving revision 1.57
> diff -u -r1.57 modperl_cmd.c
> --- src/modules/perl/modperl_cmd.c	14 Feb 2004 04:25:01 -0000	1.57
> +++ src/modules/perl/modperl_cmd.c	27 Feb 2004 16:33:23 -0000
> @@ -142,6 +142,62 @@
>      return NULL;
>  }
>  
> +MP_CMD_SRV_DECLARE(order)
> +{
> +    MP_dSCFG(parms->server);
> +    const char *name = parms->cmd->name;
> +
> +    int order;
> +
> +    /* main server only */
> +    MP_CMD_SRV_CHECK;
> +
> +    /* I tried to put these in the order of utility, thus making
> +     * a tedious task as efficient as possible
> +     */
> +    switch (*arg) {
> +      case 'R':
> +      case 'r':
> +        /* useful */
> +        if (! strcasecmp(arg, "ReallyLast")) {
> +            order = APR_HOOK_REALLY_LAST;
> +            break;
> +        }
> +        /* useful, but the default */
> +        if (! strcasecmp(arg, "ReallyFirst")) {
> +            order = APR_HOOK_REALLY_FIRST;
> +            break;
> +        }
> +      case 'L':
> +      case 'l':
> +        /* also useful */
> +        if (! strcasecmp(arg, "Last")) {
> +            order = APR_HOOK_LAST;
> +            break;
> +        }
> +      case 'F':
> +      case 'f':
> +        /* probably won't do what the user expects */
> +        if (! strcasecmp(arg, "First")) {
> +            order = APR_HOOK_FIRST;
> +            break;
> +        }
> +      case 'M':
> +      case 'm':
> +        /* probably too vague to be useful */
> +        if (! strcasecmp(arg, "Middle")) {
> +            order = APR_HOOK_MIDDLE;
> +            break;
> +        }
> +      default:
> +        return apr_pstrcat(parms->pool, "invalid value for ",
> +                           name, ": ", arg, NULL);
> +    }
> +
> +    apr_table_setn(scfg->hook_order, name, apr_itoa(parms->pool, order));

Does this ordering information need to be in the server-configuration
request since it's really a global?

> +    return NULL;
> +}
> +
>  static int modperl_vhost_is_running(server_rec *s)
>  {
>  #ifdef USE_ITHREADS
> Index: src/modules/perl/modperl_cmd.h
> ===================================================================
> RCS file: /home/cvspublic/modperl-2.0/src/modules/perl/modperl_cmd.h,v
> retrieving revision 1.22
> diff -u -r1.22 modperl_cmd.h
> --- src/modules/perl/modperl_cmd.h	9 Feb 2004 18:18:16 -0000	1.22
> +++ src/modules/perl/modperl_cmd.h	27 Feb 2004 16:33:23 -0000
> @@ -42,6 +42,7 @@
>  MP_CMD_SRV_DECLARE(load_module);
>  MP_CMD_SRV_DECLARE(set_input_filter);
>  MP_CMD_SRV_DECLARE(set_output_filter);
> +MP_CMD_SRV_DECLARE(order);
>  
>  #ifdef MP_COMPAT_1X
>  
> Index: src/modules/perl/modperl_config.c
> ===================================================================
> RCS file: /home/cvspublic/modperl-2.0/src/modules/perl/modperl_config.c,v
> retrieving revision 1.76
> diff -u -r1.76 modperl_config.c
> --- src/modules/perl/modperl_config.c	14 Feb 2004 01:38:05 -0000	1.76
> +++ src/modules/perl/modperl_config.c	27 Feb 2004 16:33:23 -0000
> @@ -179,6 +179,9 @@
>      scfg->gtop = modperl_gtop_new(p);
>  #endif        
>  
> +    /* no merge required - applies to the main server only */
> +    scfg->hook_order = apr_table_make(p, 2);
> +

Again, the hook_order should be stored globally somewhere, otherwise, we
are blowing up the server_record for no gain, imagine a setup with 1000+
vhosts for instance.

>      /* must copy ap_server_argv0, because otherwise any read/write of
>       * $0 corrupts process' argv[0] (visible with 'ps -ef' on most
>       * unices). This is due to the logic of calculating PL_origalen in
> Index: src/modules/perl/modperl_types.h
> ===================================================================
> RCS file: /home/cvspublic/modperl-2.0/src/modules/perl/modperl_types.h,v
> retrieving revision 1.73
> diff -u -r1.73 modperl_types.h
> --- src/modules/perl/modperl_types.h	12 Feb 2004 02:05:28 -0000	1.73
> +++ src/modules/perl/modperl_types.h	27 Feb 2004 16:33:23 -0000
> @@ -137,6 +137,7 @@
>      modperl_options_t *flags;
>      apr_hash_t *modules;
>      server_rec *server;
> +    MpHV *hook_order;

See above

>  } modperl_config_srv_t;
>  
>  typedef struct {
> Index: src/modules/perl/modperl_util.c
> ===================================================================
> RCS file: /home/cvspublic/modperl-2.0/src/modules/perl/modperl_util.c,v
> retrieving revision 1.62
> diff -u -r1.62 modperl_util.c
> --- src/modules/perl/modperl_util.c	12 Feb 2004 02:05:28 -0000	1.62
> +++ src/modules/perl/modperl_util.c	27 Feb 2004 16:33:23 -0000
> @@ -843,3 +843,103 @@
>      /* copy the SV in case the pool goes out of scope before the perl scalar */
>      return newSVpv(ap_server_root_relative(p, fname), 0);
>  }
> +
> +/* from here down is support for dynamic hook ordering.  this is mostly
> + * stolen from mod_info.c, so see also the logic and descriptions there.
> + */
> +
> +typedef struct
> +{
> +    void (*dummy)(void *);
> +    const char *szName;
> +    const char * const *aszPredecessors;
> +    const char * const *aszSuccessors;
> +    int nOrder;
> +} hook_struct_t;
> +
> +typedef apr_array_header_t * (
> +#ifdef WIN32
> +__stdcall
> +#endif
> +* hook_get_t)(void);
> +
> +typedef struct {
> +    const char *name;
> +    hook_get_t get;
> +} hook_lookup_t;
> +
> +static hook_lookup_t request_hooks[] = {
> +    {"PerlHookPostReadRequestHandler", ap_hook_get_post_read_request},
> +    {"PerlHookTransHandler", ap_hook_get_translate_name},
> +    {"PerlHookMapToStorageHandler", ap_hook_get_map_to_storage},
> +    {"PerlHookHeaderParserHandler", ap_hook_get_header_parser},
> +    {"PerlHookAccessHandler", ap_hook_get_access_checker},
> +    {"PerlHookAuthenHandler", ap_hook_get_check_user_id},
> +    {"PerlHookAuthzHandler", ap_hook_get_auth_checker},
> +    {"PerlHookTypeHandler", ap_hook_get_type_checker},
> +    {"PerlHookFixupHandler", ap_hook_get_fixups},
> +    {"PerlHookLogHandler", ap_hook_get_log_transaction},
> +    {NULL},
> +};
> +
> +void modperl_util_resort_hooks(modperl_config_srv_t *scfg) {
> +
> +    /* change the ordering of a specific phase, placing mod_perl someplace
> +     * than the default APR_HOOK_REALLY_FIRST order
> +     */
> +
> +    int i;
> +
> +    /* if there were no PerlHook*Handler directives we can quit early */
> +    if (apr_is_empty_table(scfg->hook_order)) {
> +        MP_TRACE_a(MP_FUNC, "hook order table is empty - using defaults");
> +        return;
> +    }
> +
> +    /* we have _something_ to process.  it would make more sense to have
> +     * scfg->hook_order drive the process, but that would require a bunch
> +     * of string comparisons to fetch the proper ap_hook_get* function...
> +     */
> +    for (i = 0; request_hooks[i].name; i++) {
> +        int int_order;
> +        const char *char_order;
> +        apr_array_header_t *hooks;
> +        hook_struct_t *elts;
> +
> +        MP_TRACE_a(MP_FUNC, "finding configured hook order for %s",
> +                   request_hooks[i].name);
> +
> +        char_order = apr_table_get(scfg->hook_order, request_hooks[i].name);
> +
> +        if (char_order == NULL) {
> +            MP_TRACE_a(MP_FUNC, "no %s specified - using defaults",
> +                       request_hooks[i].name);
> +            continue;
> +        }
> +
> +        hooks = request_hooks[i].get();
> +        elts = (hook_struct_t *)hooks->elts;
> +        int_order = atoi(char_order);
> +
> +        /* isolate mod_perl from the phase hooks and insert new ordering */
> +
> +        int j;
> +        for (j = 0; j < hooks->nelts; j++) {
> +            if (strcmp(elts[j].szName,"mod_perl.c") == 0) {
> +                if (elts[j].nOrder == MODPERL_HOOK_REALLY_FIRST) {
> +                    /* XXX hack.  don't override any of mod_perl's internal
> +                     * callbacks, just the ones users can set - szName is set
> +                     * to mod_perl.c for _every_ registered mod_perl hook.
> +                     */
> +                    continue;
> +                }
> +                MP_TRACE_a(MP_FUNC, "using %s to set hook order to %d",
> +                           request_hooks[i].name, int_order);
> +                elts[j].nOrder = int_order;
> +            }
> +        }
> +    }
> +
> +    /* resort the hooks */
> +    apr_hook_sort_all();
> +}
> Index: src/modules/perl/modperl_util.h
> ===================================================================
> RCS file: /home/cvspublic/modperl-2.0/src/modules/perl/modperl_util.h,v
> retrieving revision 1.51
> diff -u -r1.51 modperl_util.h
> --- src/modules/perl/modperl_util.h	14 Jan 2004 21:27:40 -0000	1.51
> +++ src/modules/perl/modperl_util.h	27 Feb 2004 16:33:23 -0000
> @@ -162,4 +162,6 @@
>  
>  SV *modperl_server_root_relative(pTHX_ SV *sv, const char *fname);
>  
> +void modperl_util_resort_hooks(modperl_config_srv_t *scfg);
> +
>  #endif /* MODPERL_UTIL_H */
> 
> ______________________________________________________________________
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
> For additional commands, e-mail: dev-help@perl.apache.org
-- 
--------------------------------------------------------------------------------
Philippe M. Chiasson /gozer\@(cpan|ectoplasm)\.org/ 88C3A5A5 (122FF51B/C634E37B)
http://gozer.ectoplasm.org/    F9BF E0C2 480E 7680 1AE5 3631 CB32 A107 88C3 A5A5
Q: It is impossible to make anything foolproof because fools are so ingenious.
perl -e'$$=\${gozer};{$_=unpack(P7,pack(L,$$));/^JAm_pH\n$/&&print||$$++&&redo}'