>>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
Index: Changes
===================================================================
RCS file: /home/cvs/modperl-2.0/Changes,v
retrieving revision 1.335
diff -u -r1.335 Changes
--- Changes     29 Feb 2004 05:28:43 -0000      1.335
+++ Changes     2 Mar 2004 14:01:11 -0000
@@ -12,6 +12,12 @@
 
 =item 1.99_13-dev
 
+Add new directives for manipulating the runtime order of Perl*Handlers:
+  PerlHookPostReadRequestHandler, PerlHookTransHandler,
+  PerlHookMapToStorageHandler, PerlHookHeaderParserHandler,
+  PerlHookAccessHandler, PerlHookAuthenHandler, PerlHookAuthzHandler,
+  PerlHookTypeHandler, PerlHookFixupHandler, PerlHookLogHandler
+
 Apache::PerlSections->dump() and store(filename) [Gozer]
 
 expose $c->keepalive related constants [Stas]
Index: lib/ModPerl/Code.pm
===================================================================
RCS file: /home/cvs/modperl-2.0/lib/ModPerl/Code.pm,v
retrieving revision 1.117
diff -u -r1.117 Code.pm
--- lib/ModPerl/Code.pm 20 Feb 2004 14:11:31 -0000      1.117
+++ lib/ModPerl/Code.pm 2 Mar 2004 14:01:17 -0000
@@ -626,7 +626,7 @@
 my @c_src_names = qw(interp tipool log config cmd options callback handler
                      gtop util io io_apache filter bucket mgv pcw global env
                      cgi perl perl_global perl_pp sys module svptr_table
-                     const constants apache_compat);
+                     const constants apache_compat apache);
 my @h_src_names = qw(perl_unembed);
 my @g_c_names = map { "modperl_$_" } qw(hooks directives flags xsinit);
 my @c_names   = ('mod_perl', (map "modperl_$_", @c_src_names));
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 2 Mar 2004 14:01:17 -0000
@@ -573,6 +573,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();
+
     ap_add_version_component(pconf, MP_VERSION_STRING);
     ap_add_version_component(pconf,
                              Perl_form(aTHX_ "Perl/v%vd", PL_patchlevel));
@@ -782,6 +785,26 @@
     MP_CMD_SRV_FLAG("PerlWarn", warn,
                     "Turn on -w switch"),
 #endif
+    MP_CMD_SRV_TAKE1("PerlHookPostReadRequestHandler", hook_order,
+                     "hook order for PerlPostReadRequestHandler"),
+    MP_CMD_SRV_TAKE1("PerlHookTransHandler", hook_order,
+                     "hook order for PerlTransHandler"),
+    MP_CMD_SRV_TAKE1("PerlHookMapToStorageHandler", hook_order,
+                     "hook order for PerlMapToStorageHandler"),
+    MP_CMD_SRV_TAKE1("PerlHookHeaderParserHandler", hook_order,
+                     "hook order for PerlHeaderParserHandler"),
+    MP_CMD_SRV_TAKE1("PerlHookAccessHandler", hook_order,
+                     "hook order for PerlAccessHandler"),
+    MP_CMD_SRV_TAKE1("PerlHookAuthenHandler", hook_order,
+                     "hook order for PerlAuthenHandler"),
+    MP_CMD_SRV_TAKE1("PerlHookAuthzHandler", hook_order,
+                     "hook order for PerlAuthzHandler"),
+    MP_CMD_SRV_TAKE1("PerlHookTypeHandler", hook_order,
+                     "hook order for PerlTypeHandler"),
+    MP_CMD_SRV_TAKE1("PerlHookFixupHandler", hook_order,
+                     "hook order for PerlFixupHandler"),
+    MP_CMD_SRV_TAKE1("PerlHookLogHandler", hook_order,
+                     "hook order for PerlLogHandler"),
     MP_CMD_ENTRIES,
     { NULL }, 
 }; 
Index: src/modules/perl/mod_perl.h
===================================================================
RCS file: /home/cvs/modperl-2.0/src/modules/perl/mod_perl.h,v
retrieving revision 1.63
diff -u -r1.63 mod_perl.h
--- src/modules/perl/mod_perl.h 1 Mar 2004 04:21:02 -0000       1.63
+++ src/modules/perl/mod_perl.h 2 Mar 2004 14:01:17 -0000
@@ -75,6 +75,7 @@
 #include "modperl_perl.h"
 #include "modperl_svptr_table.h"
 #include "modperl_module.h"
+#include "modperl_apache.h"
 
 int modperl_init_vhost(server_rec *s, apr_pool_t *p,
                        server_rec *base_server);
Index: src/modules/perl/modperl_cmd.c
===================================================================
RCS file: /home/cvs/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      2 Mar 2004 14:01:17 -0000
@@ -142,6 +142,63 @@
     return NULL;
 }
 
+MP_CMD_SRV_DECLARE(hook_order)
+{
+    const char *name = parms->cmd->name;
+
+    int order;
+    apr_table_t *hook_order = modperl_apache_get_hook_order(parms->pool);
+
+    /* 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(hook_order, name, apr_itoa(parms->pool, order));
+
+    return NULL;
+}
+
 static int modperl_vhost_is_running(server_rec *s)
 {
 #ifdef USE_ITHREADS
Index: src/modules/perl/modperl_cmd.h
===================================================================
RCS file: /home/cvs/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      2 Mar 2004 14:01:17 -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(hook_order);
 
 #ifdef MP_COMPAT_1X
 
Index: todo/features_missing
===================================================================
RCS file: /home/cvs/modperl-2.0/todo/features_missing,v
retrieving revision 1.3
diff -u -r1.3 features_missing
--- todo/features_missing       29 Feb 2004 05:28:43 -0000      1.3
+++ todo/features_missing       2 Mar 2004 14:01:18 -0000
@@ -28,9 +28,6 @@
 
 * Apache::src: tobe a compat stub around new build framework
 
-* hooks ordering is not implemented yet
-  Owner: geoff
-
 * tied filehandle interface:
  -EOF, TELL, SEEK
  -READLINE - proper implementation (see comment in Apache::compat)
Index: xs/tables/current/ModPerl/FunctionTable.pm
===================================================================
RCS file: /home/cvs/modperl-2.0/xs/tables/current/ModPerl/FunctionTable.pm,v
retrieving revision 1.146
diff -u -r1.146 FunctionTable.pm
--- xs/tables/current/ModPerl/FunctionTable.pm  14 Feb 2004 17:00:26 -0000      1.146
+++ xs/tables/current/ModPerl/FunctionTable.pm  2 Mar 2004 14:01:24 -0000
@@ -7085,6 +7085,21 @@
       }
     ]
   },
+  {
+    'return_type' => 'apr_table_t *',
+    'name' => 'modperl_apache_get_hook_order',
+    'args' => [
+      {
+        'type' => 'apr_pool_t *',
+        'name' => 'pool'
+      },
+    ]
+  },
+  {
+    'return_type' => 'void',
+    'name' => 'modperl_apache_resort_hooks',
+    'args' => []
+  },
 ];
 
 

--- /dev/null   2003-01-30 05:24:37.000000000 -0500
+++ src/modules/perl/modperl_apache.c   2004-03-01 15:04:29.000000000 -0500
@@ -0,0 +1,116 @@
+#include "mod_perl.h"
+
+/* PerlHook*Handler support */
+static apr_table_t *hook_order;
+
+apr_table_t *modperl_apache_get_hook_order(apr_pool_t *pool) {
+
+    if (!hook_order) {
+        hook_order = apr_table_make(pool, 0);
+    }
+
+    return hook_order;
+}
+
+/* these typedefs (and descriptions) taken from mod_info.c as of 2.0.49 */
+
+typedef struct { /*XXX: should get something from apr_hooks.h instead */
+    void (*pFunc)(void); /* just to get the right size */
+    const char *szName;
+    const char * const *aszPredecessors;
+    const char * const *aszSuccessors;
+    int nOrder;
+} hook_struct_t;
+
+/*
+ * hook_get_t is a pointer to a function that takes void as an argument and
+ * returns a pointer to an apr_array_header_t.  The nasty WIN32 ifdef
+ * is required to account for the fact that the ap_hook* calls all use
+ * STDCALL calling convention.
+ */
+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},
+};
+
+/* PerlHook*Handler support */
+void modperl_apache_resort_hooks() {
+
+    /* change the ordering of a specific phase, placing mod_perl someplace
+     * than the default APR_HOOK_REALLY_FIRST order
+     */
+
+    int i, j;
+
+    /* if there were no PerlHook*Handler directives we can quit early */
+    if (apr_is_empty_table(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
+     * the hook_order table 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(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 */
+
+        for (j = 0; j < hooks->nelts; j++) {
+            if (strcmp(elts[j].szName,"mod_perl.c") == 0) {
+                if (elts[j].nOrder == MODPERL_HOOK_REALLY_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();
+}

--- /dev/null   2003-01-30 05:24:37.000000000 -0500
+++ src/modules/perl/modperl_apache.h   2004-03-01 15:04:44.000000000 -0500
@@ -0,0 +1,9 @@
+#ifndef MODPERL_APACHE_H
+#define MODPERL_APACHE_H
+
+/* PerlHook*Handler support */
+apr_table_t *modperl_apache_get_hook_order(apr_pool_t *pool);
+
+void modperl_apache_resort_hooks(void);
+
+#endif /* MODPERL_UTIL_H */

---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]

Reply via email to