You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@perl.apache.org by Stas Bekman <st...@stason.org> on 2003/04/10 10:53:31 UTC

[patch] filter_init implementation

OK, I've an unpolished implementation of the filter_init functionality
and a test (attached and inlined at the end).

There are several issue to discuss:

1) Look at the TestFilter::in_init_basic test. The interesting chunks
are:

sub init : FilterInitHandler {
     my $filter = shift;
     ...
     return Apache::OK;
}

sub filt : FilterRequestHandler
                   FilterNeedsHandler(TestFilter::in_init_basic::init)
     {
     my ($filter, $bb) = @_;
     ...
}

If we remove the code and concentrate on the attributes we now have:

sub init : FilterInitHandler {}
sub filt : FilterRequestHandler FilterNeedsHandler(init) {}

Originally I thought that we should use:

sub init {}
sub filt : FilterRequestHandler FilterInitHandler(init)  {}

but it using the FilterInitHandler attr in the non-init sub sounded
like a misleading attribute name, so I've renamed it to
'FilterNeedsHandler' and used the 'FilterInitHandler' in the init sub
itself, so we can even verify that the assigned init sub was actually
designed to be such (the actual check is commented out in the patch).

Does it look good? or do you prefer it differently?

Maybe one of these:

   s/FilterNeedsHandler/FilterNeedsInitHandler/
   s/FilterNeedsHandler/FilterRunsInitHandler/

2) the init sub is run immediately after the filter handler is pushed
onto the filter stack, which is about the same time as the core
filter_init is executed.

However, it's always run for connection filters, whereas in the httpd
land connection filters are initialized only for HTTP requests. I
believe we are doing better than httpd. I don't think we will have a
problem here. Correct me if I'm wrong.

3) the issue with the argument to FilterNeedsHandler(), at this moment
it's just passed as '(whatever)' string, '(' and ')' are removed and
the remaining string is used as a fully-qualified function. I guess we
could be more elaborate here and accept a fully blown perl code, like
Attribute::Handlers does, but it'll require eval() which can't be done
when the attributes are parsed, because the code is not compiled yet
(we do some sort of source filtering here). So at this moment use in
your tests the fully qualified function without any quotes, as in:

   FilterNeedsHandler(TestFilter::in_init_basic::init)

4) Geoff, if you can now write a better test that actually does
something useful please send it in.

---------

the patch:

Index: src/modules/perl/modperl_filter.c
===================================================================
RCS file: /home/cvs/modperl-2.0/src/modules/perl/modperl_filter.c,v
retrieving revision 1.60
diff -u -r1.60 modperl_filter.c
--- src/modules/perl/modperl_filter.c	8 Apr 2003 07:41:51 -0000	1.60
+++ src/modules/perl/modperl_filter.c	10 Apr 2003 08:29:38 -0000
@@ -253,6 +253,38 @@
      return mg ? (modperl_filter_t *)mg->mg_ptr : NULL;
  }

+static int modperl_run_filter_init(ap_filter_t *f,
+                                   modperl_handler_t *handler)
+{
+    AV *args = Nullav;
+    int status;
+
+    request_rec *r = f->r;
+    conn_rec    *c = f->c;
+    server_rec  *s = r ? r->server : c->base_server;
+    apr_pool_t  *p = r ? r->pool : c->pool;
+
+    MP_dINTERP_SELECT(r, c, s);
+
+    modperl_handler_make_args(aTHX_ &args,
+                              "Apache::Filter", f,
+                              NULL);
+
+    //modperl_filter_mg_set(aTHX_ AvARRAY(args)[0], filter);
+
+    if ((status = modperl_callback(aTHX_ handler, p, r, s, args)) != OK) {
+        status = modperl_errsv(aTHX_ status, r, s);
+    }
+
+    SvREFCNT_dec((SV*)args);
+
+    MP_TRACE_f(MP_FUNC, MP_FILTER_NAME_FORMAT
+               "return: %d\n", handler->name, status);
+
+    return status;
+}
+
+
  int modperl_run_filter(modperl_filter_t *filter)
  {
      AV *args = Nullav;
@@ -692,6 +724,64 @@
  }


+static int modperl_filter_do_filter_init_handler(ap_filter_t *f,
+                                                 modperl_handler_t *handler,
+                                                 apr_pool_t *p,
+                                                 server_rec *s)
+{
+    dTHX; /* XXX: make an argument */
+    char *init_handler_name;
+    CV *cv;
+    int status = OK;
+
+    if (handler->mgv_cv) {
+        GV *gv;
+        if ((gv = modperl_mgv_lookup(aTHX_ handler->mgv_cv))) {
+            cv = modperl_mgv_cv(gv);
+        }
+    }
+
+    if (cv && SvMAGICAL(cv)) {
+        MAGIC *mg = mg_find((SV*)(cv), '~');
+        init_handler_name = mg ? mg->mg_ptr : NULL;
+    }
+    else {
+        return status;
+    }
+
+    if (init_handler_name) {
+        modperl_handler_t *init_handler =
+            modperl_handler_new(p, apr_pstrdup(p, init_handler_name));
+
+        /* XXX: probably could resolve earlier, but at the
+         * time the attributes are parsed the init sub could
+         * be still unparsed and therefore triggering a
+         * resolve during parsin is a bad idea. see if can do
+         * it once (e.g. cache) */
+        status = modperl_handler_resolve(aTHX_ &init_handler, p, s);
+        if (status != OK) {
+            return status;
+        }
+
+        MP_TRACE_h(MP_FUNC,
+                   "running filter init handler %s\n",
+                   init_handler->name);
+
+/*         XXX: should we check that the init_handler has the */
+/*         attribute FilterInitHandler set? */
+/*         if (! init_handler->attrs & MP_FILTER_INIT_HANDLER) { */
+/*             Perl_croak(aTHX_ "handler %s doesn't have " */
+/*                        "the FilterInitHandler attribute set", */
+/*                        init_handler->name); */
+/*         } */
+
+        return modperl_run_filter_init(f, init_handler);
+    }
+
+    return status;
+}
+
+
  static int modperl_filter_add_connection(conn_rec *c,
                                           int idx,
                                           const char *name,
@@ -701,10 +791,12 @@
      modperl_config_dir_t *dcfg =
          modperl_config_dir_get_defaults(c->base_server);
      MpAV *av;
-
+    int status;
+
      if ((av = dcfg->handlers_per_dir[idx])) {
          modperl_handler_t **handlers = (modperl_handler_t **)av->elts;
          int i;
+        ap_filter_t *f;

          for (i=0; i<av->nelts; i++) {
              modperl_filter_ctx_t *ctx;
@@ -726,8 +818,15 @@

              ctx = (modperl_filter_ctx_t *)apr_pcalloc(c->pool, sizeof(*ctx));
              ctx->handler = handlers[i];
-            addfunc(name, (void*)ctx, NULL, c);

+            f = addfunc(name, (void*)ctx, NULL, c);
+            status = modperl_filter_do_filter_init_handler(f, handlers[i],
+                                                           c->pool,
+                                                           c->base_server);
+            if (status != OK) {
+                return status;
+            }
+
              MP_TRACE_h(MP_FUNC, "%s handler %s configured (connection)\n",
                         type, handlers[i]->name);
          }
@@ -749,6 +848,7 @@
  {
      MP_dDCFG;
      MpAV *av;
+    int status;

      if ((av = dcfg->handlers_per_dir[idx])) {
          modperl_handler_t **handlers = (modperl_handler_t **)av->elts;
@@ -799,8 +899,15 @@

              ctx = (modperl_filter_ctx_t *)apr_pcalloc(r->pool, sizeof(*ctx));
              ctx->handler = handlers[i];
-            addfunc(name, (void*)ctx, r, r->connection);

+            f = addfunc(name, (void*)ctx, r, r->connection);
+
+            status = modperl_filter_do_filter_init_handler(f, handlers[i],
+                                                           r->pool, r->server);
+            if (status != OK) {
+                return status;
+            }
+
              MP_TRACE_h(MP_FUNC, "%s handler %s configured (%s)\n",
                         type, handlers[i]->name, r->uri);
          }
Index: src/modules/perl/modperl_filter.h
===================================================================
RCS file: /home/cvs/modperl-2.0/src/modules/perl/modperl_filter.h,v
retrieving revision 1.22
diff -u -r1.22 modperl_filter.h
--- src/modules/perl/modperl_filter.h	3 Apr 2003 06:23:53 -0000	1.22
+++ src/modules/perl/modperl_filter.h	10 Apr 2003 08:29:38 -0000
@@ -9,7 +9,8 @@

  #define MP_FILTER_CONNECTION_HANDLER 0x01
  #define MP_FILTER_REQUEST_HANDLER    0x02
-#define MP_FILTER_HTTPD_HANDLER      0x04
+#define MP_FILTER_INIT_HANDLER       0x04
+#define MP_FILTER_HTTPD_HANDLER      0x08

  typedef ap_filter_t * MP_FUNC_T(modperl_filter_add_t) (const char *, void *,
                                                         request_rec *,
Index: xs/Apache/Filter/Apache__Filter.h
===================================================================
RCS file: /home/cvs/modperl-2.0/xs/Apache/Filter/Apache__Filter.h,v
retrieving revision 1.28
diff -u -r1.28 Apache__Filter.h
--- xs/Apache/Filter/Apache__Filter.h	4 Apr 2003 00:08:37 -0000	1.28
+++ xs/Apache/Filter/Apache__Filter.h	10 Apr 2003 08:29:38 -0000
@@ -76,6 +76,23 @@
  #define trace_attr()
  #endif

+/* eval q/(\&init)/ or eval q/('init')/ */
+//#define
+static void add_magic(pTHX_ SV *cv, char *string, STRLEN len)
+{
+    /* XXX: better parsing */
+    char *str;
+    len -= 2; /* s/ \( | \) //x */
+    string++; /* skip the first ( */
+    New(0, str, len+1, char);
+    Copy(string, str, len+1, char);
+    str[len] = '\0';
+    fprintf(stderr, "string PV: [%s]\n", str);
+    sv_magic(cv, Nullsv, '~', NULL, -1);
+    SvMAGIC(cv)->mg_ptr = (char *)str;
+}
+
+
  static XS(MPXS_modperl_filter_attributes)
  {
      dXSARGS;
@@ -101,6 +118,21 @@
                  trace_attr();
                  continue;
              }
+          case 'I':
+            if (strEQ(pv, "InitHandler")) {
+                *attrs |= MP_FILTER_INIT_HANDLER;
+                trace_attr();
+                continue;
+            }
+          case 'N':
+            if (strnEQ(pv, "NeedsHandler", 12)) {
+                pv += 12;
+                //STRLEN sub_len =  (len - (pv - attribute)
+                fprintf(stderr, "pv: %s, len: %d\n", pv, len - (pv - 
attribute) );
+                add_magic(aTHX_ SvRV(ST(1)), pv, len - (pv - attribute));
+                trace_attr();
+                continue;
+            }
            case 'R':
              if (strEQ(pv, "RequestHandler")) {
                  *attrs |= MP_FILTER_REQUEST_HANDLER;
@@ -108,6 +140,7 @@
                  continue;
              }
            default:
+         /* XXX: there could be more than one attr to pass through */
              XPUSHs_mortal_pv(attribute);
              XSRETURN(1);
          }
--- /dev/null	1970-01-01 10:00:00.000000000 +1000
+++ t/filter/TestFilter/in_init_basic.pm	2003-04-10 18:28:22.000000000 +1000
@@ -0,0 +1,71 @@
+package TestFilter::in_init_basic;
+
+use strict;
+use warnings FATAL => 'all';
+
+use Apache::Test;
+use Apache::TestUtil;
+
+use Apache::RequestRec ();
+use Apache::RequestIO ();
+
+use base qw(Apache::Filter);
+
+use Apache::Const -compile => qw(OK M_POST);
+
+use constant READ_SIZE  => 1024;
+
+# this filter is expected to be called once
+# it'll set a note, with the count
+sub init : FilterInitHandler {
+    my $filter = shift;
+
+    warn "init was invoked\n";
+
+    my $ctx = $filter->ctx;
+    $ctx->{init}++;
+    $filter->r->notes->set(init => $ctx->{init});
+    $filter->ctx($ctx);
+
+    return Apache::OK;
+}
+
+# this filter passes the data through unmodified and sets a note
+# counting how many times it was invoked
+sub transparent : FilterRequestHandler
+                  FilterNeedsHandler(TestFilter::in_init_basic::init)
+    {
+    my ($filter, $bb) = @_;
+
+    my $ctx = $filter->ctx;
+    $ctx->{run}++;
+    $filter->r->notes->set(run => $ctx->{run});
+    $filter->ctx($ctx);
+
+    my $rv = $filter->next->pass_brigade($bb);
+    return $rv unless $rv == APR::SUCCESS;
+
+    return Apache::OK;
+}
+
+sub response {
+    my $r = shift;
+
+    $r->content_type('text/plain');
+
+    if ($r->method_number == Apache::M_POST) {
+        $r->print(ModPerl::Test::read_post($r));
+    }
+
+    my @keys = qw(init run);
+    my %times = map { $_ => $r->notes->get($_)||0 } @keys;
+    $r->print("$_ $times{$_}\n") for @keys;
+
+    Apache::OK;
+}
+1;
+__DATA__
+SetHandler modperl
+PerlModule             TestFilter::in_init_basic
+PerlResponseHandler    TestFilter::in_init_basic::response
+PerlInputFilterHandler TestFilter::in_init_basic::transparent
--- /dev/null	1970-01-01 10:00:00.000000000 +1000
+++ t/filter/in_init_basic.t	2003-04-10 16:24:40.000000000 +1000
@@ -0,0 +1,15 @@
+use strict;
+use warnings FATAL => 'all';
+
+use Apache::Test;
+use Apache::TestRequest;
+use Apache::TestUtil;
+
+plan tests => 1;
+
+my $content = "content ok\n";
+my $expected = join '', $content, "init 1\n", "run 2\n";
+
+my $location = '/TestFilter::in_init_basic';
+my $response = POST_BODY $location, content => $content;
+ok t_cmp($expected, $response, "test filter init functionality");



__________________________________________________________________
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

Re: [patch] filter_init implementation

Posted by Stas Bekman <st...@stason.org>.
Geoffrey Young wrote:
>> however, the test dumps core for me.  below is a backtrace, test 
>> output, and error_log snippet.
> 
> 
> ok, I couldn't get the input filter init to work, but I did write a 
> simple output filter test (attached)

Hmm, it's an interesting diff. how did you create this patch? what's foo? I 
had to 'perl -pi -e 's|foo|/dev/null|' to be able to use it as a patch.

> the test does not yeild what I would expect - I would expect to see the 
> content ('init 1') printed before the filter is called.  The first 
> output filter isn't generally called before there is any content, is it?

First of all, you had a tiny bug which wasn't noticed:

replace:

FilterNeedsHandler(TestFilter::in_init_basic::init)

with:

FilterNeedsHandler(TestFilter::out_init_basic::init)

so of course you were calling TestFilter::in_init_basic::init from my test.

Second, you wanted to test that 'init 1' comes before any 'content ok' is 
sent, right? So you should organize the response handler to do so. I'd even 
move that into the filter handler where it prints 'run 1'.

Anyways, I've changed a bit the expectations and the response handler and 
voila, it works ;)

the new version is attached.

__________________________________________________________________
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

Re: [patch] filter_init implementation

Posted by Geoffrey Young <ge...@modperlcookbook.org>.
> however, the test dumps core for me.  below is a backtrace, 
> test output, and error_log snippet.

ok, I couldn't get the input filter init to work, but I did write a simple 
output filter test (attached)

the test does not yeild what I would expect - I would expect to see the 
content ('init 1') printed before the filter is called.  The first output 
filter isn't generally called before there is any content, is it?

anyway, I hope this helps you a bit.

--Geoff

# testing : test filter init functionality
# expected: 'content ok
# init 1
# run 1
# run 2
# run 3
# '
# received: 'content ok
# run 1
# init 1
# run 2
# run 3
# '



Re: [patch] filter_init implementation

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

Stas Bekman wrote:
> OK, I have completed the fully blown perl support and cleaned up the 
> implementation.

whee!

> 
> Now it should be pretty easy to add support for more than one init 
> handler (now only 1 is supported), as I've changed the modperl_handler_t 
> struct to be a chain of handlers. so if we get two 
> FilterHasInitHandler() attrs for the same filter we can add them twice 
> to the cv's magic field and then later push them all into the parent 
> handler's chain. However for now I don't see a need for this 
> functionality. Let me know if you think we need it.

I think it would be nice to have it ('if you build it, they will come'), but 
I don't see it as big issue right now.

> 
> Also it should be pretty easy to add support for other attributes which 
> accept perl code as their arguments. This can be useful for other handlers.
> 
> At the end I've decided to use FilterInitHandler and 
> FilterHasInitHandler attribute names. Let's see if after using them for 
> a bit it feels good. I decided to keep the init concept, since pre 
> without a noun doesn't sound good, whereas init is OK. Moreover, we 
> (mod_perl) *are* implementing an filter_init and we run it for request 
> and connection filters, for HTTP and any other protocol, whereas Apache 
> does that only for HTTP.

that sounds fine to me.

> 
> I've put some preliminary docs here (the url will autovivify within 6h):
> http://perl.apache.org/docs/2.0/api/Apache/Filter.html
> 
> Please take a look and patch it if it reads unclear... Thanks.

looks good to me.  I'll have some examples for everyone 'soonish'.

--Geoff


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


Re: [patch] filter_init implementation

Posted by Stas Bekman <st...@stason.org>.
OK, I have completed the fully blown perl support and cleaned up the 
implementation.

Now it should be pretty easy to add support for more than one init handler 
(now only 1 is supported), as I've changed the modperl_handler_t struct to be 
a chain of handlers. so if we get two FilterHasInitHandler() attrs for the 
same filter we can add them twice to the cv's magic field and then later push 
them all into the parent handler's chain. However for now I don't see a need 
for this functionality. Let me know if you think we need it.

Also it should be pretty easy to add support for other attributes which accept 
perl code as their arguments. This can be useful for other handlers.

At the end I've decided to use FilterInitHandler and FilterHasInitHandler 
attribute names. Let's see if after using them for a bit it feels good. I 
decided to keep the init concept, since pre without a noun doesn't sound good, 
whereas init is OK. Moreover, we (mod_perl) *are* implementing an filter_init 
and we run it for request and connection filters, for HTTP and any other 
protocol, whereas Apache does that only for HTTP.

I've put some preliminary docs here (the url will autovivify within 6h):
http://perl.apache.org/docs/2.0/api/Apache/Filter.html

Please take a look and patch it if it reads unclear... 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: [patch] filter_init implementation

Posted by Geoffrey Young <ge...@modperlcookbook.org>.
>>  > The question still stands. Do we want:
>>  >
>>  > sub init {}
>>  > sub filt : FilterRequestHandler FilterPreHandler(init)  {}
>>  >
>>  > or
>>  >
>>  > sub init : FilterPreHandler {}
>>  > sub filt : FilterRequestHandler FilterNeedsPreHandler(init) {}
>>
>> the first seems to make the most sense to me - I don't see why we need 
>> to make the pre handler take an attribute only to create another 
>> attribute to run it.
> 
> 
> Well, I agree with you. It's just that I don't feel very happy about 
> linguistics. Currently attributes indentify the properties of the 
> filters. FilterPreHandler(init) has nothing to do with the real filter, 
> other than creating a dependency. 

well, for some it can really be considered part of the filter itself, 
since the filter may not behave correctly without it :)

> But I guess it's just a matter of 
> getting used to it. However if you see what I mean, won't 
> FilterRunsPreHandler(init) sound more like a property:
> 
> sub filt : FilterRequestHandler FilterRunsPreHandler(init) {}
> 
> So in English we can say that filter 'filt' is a request filter handler 
> that runs prehandler 'init'.
> 
> Whereas FilterPreHandler can thought as being the prehandler.
> 
> See what I mean?

yeah.  I guess I can see both sides and don't have any strong opinions 
either way.  or maybe I'm starting to like what you just said more, 
making each an attribute.  I like FilterRunsPreHandler better than 
NeedsPreHandler, in either case.

> 
> Also:
> 
> sub init : FilterPreHandler {}
> 
> makes it sure that that sub won't be used in any other way, than 
> designed. But that could be redundant I suppose.

maybe that kind of protection is useful.  I was thinking that other 
handlers may want to share the logic within the init handler, but as I 
think of it more it seems unlikely.

> 
>>  > Anyways, I've changed a bit the expectations and the response handler
>>  > and voila, it works ;)
>>  >
>>  > the new version is attached.
>>
>> ah, cool.  the output filter test worked too.
>>
>> I'd say commit all this stuff - we certainly have enough to work with 
>> now, and we can build on it as we go.
> 
> 
> Looking forward to see your conditional GET tests and may be other 
> interesting apps.

yeah, as I work out the details of conditional GETs I'll add some 
tests.  it will be a while, though - I still have some thinking to do.

> 
>> very nice work, stas.
> 
> 
> ;)
> 
> I still have to complete the source parsing to support fully blown perl 
> code as an argument to FilterPreHandler().

no rest for the weary :)

--Geoff


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


Re: [patch] filter_init implementation

Posted by Stas Bekman <st...@stason.org>.
Geoffrey Young wrote:
> 
>> Hmm, I'm puzzled how did it work for me at all! The filter handler was 
>> bogus. Do you still have a segfault if you replace it with:
> 
> 
> yes, that worked.  nice.

Great!

>  >> FilterPreHandler?  it's really preprocessing that is going on here, so
>  >> maybe that's more descriptive than anything 'init' based.  I agree now
>  >> more than ever that filter_init is a really bad name, and will have
>  >> misleading connotations for Perl developers, who expect different
>  >> things from initialization routines :)
>  >
>  > Sounds fine with me. So we do design a new phase missing from Apache ;)
>  > Those who want to do filter initialization can use $f->ctx.
>  >
>  > The question still stands. Do we want:
>  >
>  > sub init {}
>  > sub filt : FilterRequestHandler FilterPreHandler(init)  {}
>  >
>  > or
>  >
>  > sub init : FilterPreHandler {}
>  > sub filt : FilterRequestHandler FilterNeedsPreHandler(init) {}
> 
> the first seems to make the most sense to me - I don't see why we need 
> to make the pre handler take an attribute only to create another 
> attribute to run it.

Well, I agree with you. It's just that I don't feel very happy about 
linguistics. Currently attributes indentify the properties of the filters. 
FilterPreHandler(init) has nothing to do with the real filter, other than 
creating a dependency. But I guess it's just a matter of getting used to it. 
However if you see what I mean, won't FilterRunsPreHandler(init) sound more 
like a property:

sub filt : FilterRequestHandler FilterRunsPreHandler(init) {}

So in English we can say that filter 'filt' is a request filter handler that 
runs prehandler 'init'.

Whereas FilterPreHandler can thought as being the prehandler.

See what I mean?

Also:

sub init : FilterPreHandler {}

makes it sure that that sub won't be used in any other way, than designed. But 
that could be redundant I suppose.

>  > Anyways, I've changed a bit the expectations and the response handler
>  > and voila, it works ;)
>  >
>  > the new version is attached.
> 
> ah, cool.  the output filter test worked too.
> 
> I'd say commit all this stuff - we certainly have enough to work with 
> now, and we can build on it as we go.

Looking forward to see your conditional GET tests and may be other interesting 
apps.

> very nice work, stas.

;)

I still have to complete the source parsing to support fully blown perl code 
as an argument to FilterPreHandler().

__________________________________________________________________
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: [patch] filter_init implementation

Posted by Geoffrey Young <ge...@modperlcookbook.org>.
> Hmm, I'm puzzled how did it work for me at all! The filter handler was 
> bogus. Do you still have a segfault if you replace it with:

yes, that worked.  nice.

 >> FilterPreHandler?  it's really preprocessing that is going on here, so
 >> maybe that's more descriptive than anything 'init' based.  I agree now
 >> more than ever that filter_init is a really bad name, and will have
 >> misleading connotations for Perl developers, who expect different
 >> things from initialization routines :)
 >
 >
 > Sounds fine with me. So we do design a new phase missing from Apache ;)
 > Those who want to do filter initialization can use $f->ctx.
 >
 > The question still stands. Do we want:
 >
 > sub init {}
 > sub filt : FilterRequestHandler FilterPreHandler(init)  {}
 >
 > or
 >
 > sub init : FilterPreHandler {}
 > sub filt : FilterRequestHandler FilterNeedsPreHandler(init) {}

the first seems to make the most sense to me - I don't see why we need to 
make the pre handler take an attribute only to create another attribute to 
run it.

 > Anyways, I've changed a bit the expectations and the response handler
 > and voila, it works ;)
 >
 > the new version is attached.

ah, cool.  the output filter test worked too.

I'd say commit all this stuff - we certainly have enough to work with now, 
and we can build on it as we go.

very nice work, stas.

--Geoff


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


Re: [patch] filter_init implementation

Posted by Stas Bekman <st...@stason.org>.
Stas Bekman wrote:
> Geoffrey Young wrote:
> 
>>
>>
>> Stas Bekman wrote:
>>
>>> OK, I've an unpolished implementation of the filter_init functionality
>>> and a test (attached and inlined at the end).
>>
>>
>>
>> coolio.  however, the test dumps core for me.  below is a backtrace, 
>> test output, and error_log snippet.
> 
> 
> I've seen this before in a different context. Though I doubt this has 
> anything to do with filter_init, but rather the actual filter itself. 
> Can you send in a complete bug report so I can try to reproduce your 
> setup? Of course if you can debug the problem it's even better ;)
> 
> The segfault happens because Apache asserts. Would be nice to find out a 
> more graceful way to die, rather than segfault.

Hmm, I'm puzzled how did it work for me at all! The filter handler was bogus. 
Do you still have a segfault if you replace it with:

sub transparent : FilterRequestHandler
                   FilterNeedsHandler(TestFilter::in_init_basic::init)
     {
     my ($filter, $bb, $mode, $block, $readbytes) = @_;

     my $ctx = $filter->ctx;
     $ctx->{run}++;
     $filter->r->notes->set(run => $ctx->{run});
     $filter->ctx($ctx);

     my $rv = $filter->next->get_brigade($bb, $mode, $block, $readbytes);
     return $rv unless $rv == APR::SUCCESS;

     return Apache::OK;
}

obviously it had to get_brigade and not pass_brigade, since it's an input filter.

__________________________________________________________________
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: [patch] filter_init implementation

Posted by Stas Bekman <st...@stason.org>.
Geoffrey Young wrote:
> 
> 
> Stas Bekman wrote:
> 
>> OK, I've an unpolished implementation of the filter_init functionality
>> and a test (attached and inlined at the end).
> 
> 
> coolio.  however, the test dumps core for me.  below is a backtrace, 
> test output, and error_log snippet.

I've seen this before in a different context. Though I doubt this has anything 
to do with filter_init, but rather the actual filter itself. Can you send in a 
complete bug report so I can try to reproduce your setup? Of course if you can 
debug the problem it's even better ;)

The segfault happens because Apache asserts. Would be nice to find out a more 
graceful way to die, rather than segfault.

>> but it using the FilterInitHandler attr in the non-init sub sounded
>> like a misleading attribute name, so I've renamed it to
>> 'FilterNeedsHandler' and used the 'FilterInitHandler' in the init sub
>> itself, so we can even verify that the assigned init sub was actually
>> designed to be such (the actual check is commented out in the patch).
>>
>> Does it look good? or do you prefer it differently?
>>
>> Maybe one of these:
>>
>>   s/FilterNeedsHandler/FilterNeedsInitHandler/
>>   s/FilterNeedsHandler/FilterRunsInitHandler/
> 
> 
> FilterPreHandler?  it's really preprocessing that is going on here, so 
> maybe that's more descriptive than anything 'init' based.  I agree now 
> more than ever that filter_init is a really bad name, and will have 
> misleading connotations for Perl developers, who expect different things 
> from initialization routines :)

Sounds fine with me. So we do design a new phase missing from Apache ;)
Those who want to do filter initialization can use $f->ctx.

The question still stands. Do we want:

sub init {}
sub filt : FilterRequestHandler FilterPreHandler(init)  {}

or

sub init : FilterPreHandler {}
sub filt : FilterRequestHandler FilterNeedsPreHandler(init) {}


>> 2) the init sub is run immediately after the filter handler is pushed
>> onto the filter stack, which is about the same time as the core
>> filter_init is executed.
> 
> 
> as long as it is before any content handlers run for output filters, 
> that's ok.

this is true for filters added before the response handler is run, those added 
from within a response handler don't fit the bill. (actually I need to 
implement this bit for these in modperl_filter_runtime_add)


__________________________________________________________________
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: [patch] filter_init implementation

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

Stas Bekman wrote:
> OK, I've an unpolished implementation of the filter_init functionality
> and a test (attached and inlined at the end).

coolio.  however, the test dumps core for me.  below is a backtrace, test 
output, and error_log snippet.

> but it using the FilterInitHandler attr in the non-init sub sounded
> like a misleading attribute name, so I've renamed it to
> 'FilterNeedsHandler' and used the 'FilterInitHandler' in the init sub
> itself, so we can even verify that the assigned init sub was actually
> designed to be such (the actual check is commented out in the patch).
> 
> Does it look good? or do you prefer it differently?
> 
> Maybe one of these:
> 
>   s/FilterNeedsHandler/FilterNeedsInitHandler/
>   s/FilterNeedsHandler/FilterRunsInitHandler/

FilterPreHandler?  it's really preprocessing that is going on here, so maybe 
that's more descriptive than anything 'init' based.  I agree now more than 
ever that filter_init is a really bad name, and will have misleading 
connotations for Perl developers, who expect different things from 
initialization routines :)

> 
> 2) the init sub is run immediately after the filter handler is pushed
> onto the filter stack, which is about the same time as the core
> filter_init is executed.

as long as it is before any content handlers run for output filters, that's ok.

> 
> 4) Geoff, if you can now write a better test that actually does
> something useful please send it in.

yes.  once I get the test working I'll take a look at writing some 
additional tests and tweaking it a bit.

--Geoff

[geoff@mainsheet modperl-2.0]$ t/TEST t/filter/in_init_basic.t -v
*** setting ulimit to allow core files
[snip]
filter/in_init_basic....1..1
# Running under perl version 5.009 for linux
# Current time local: Thu Apr 10 09:26:29 2003
# Current time GMT:   Thu Apr 10 13:26:29 2003
# Using Test.pm version 1.23
# testing : test filter init functionality
# expected: 'content ok
# init 1
# run 2
# '
# received: ''
not ok 1
# Failed test 1 in filter/in_init_basic.t at line 15
FAILED test 1
         Failed 1/1 tests, 0.00% okay
Failed Test            Stat Wstat Total Fail  Failed  List of Failed
-------------------------------------------------------------------------------
filter/in_init_basic.t                1    1 100.00%  1
*** server localhost:8529 shutdown
*** port 8529 still in use...
done
!!! error running tests (please examine t/logs/error_log)
!!! oh gosh, server dumped core
!!! for stacktrace, run: gdb /usr/local/apache2/bin/httpd -core 
/src/modperl-2.0/t/core

#0  0x4014dd41 in __kill () from /lib/libc.so.6
#1  0x40124217 in raise (sig=6) at signals.c:65
#2  0x4014f0d8 in abort () at ../sysdeps/generic/abort.c:88
#3  0x80887d4 in piped_log_spawn (pl=0x80a65c0) at log.c:723
#4  0x8071120 in ap_get_client_block (r=0x8cf8260, buffer=0x8d06e48 "", 
bufsiz=8192) at http_protocol.c:1926
#5  0x4043a5fd in mpxs_ap_get_client_block (my_perl=0x886f618, r=0x8cf8260, 
buffer=0x84b3cd8, bufsiz=8192)
     at /src/modperl-2.0/xs/Apache/RequestIO/Apache__RequestIO.h:164
#6  0x4043b0a7 in XS_Apache__RequestRec_get_client_block (my_perl=0x886f618, 
cv=0x867c944) at RequestIO.xs:45
#7  0x403271dd in Perl_pp_entersub (my_perl=0x886f618) at pp_hot.c:2733
#8  0x402ffa65 in Perl_runops_debug (my_perl=0x886f618) at dump.c:1424
#9  0x402a408f in S_call_body (my_perl=0x886f618, myop=0xbffff520, 
is_eval=0) at perl.c:1945
#10 0x402a3bd7 in Perl_call_sv (my_perl=0x886f618, sv=0x84e56f0, flags=4) at 
perl.c:1863
#11 0x40287268 in modperl_callback (my_perl=0x886f618, handler=0x81379d8, 
p=0x8cf8228, r=0x8cf8260, s=0x80c47c8, args=0x8cf0570)
     at modperl_callback.c:53
#12 0x40287b3b in modperl_callback_run_handlers (idx=6, type=4, r=0x8cf8260, 
c=0x0, s=0x80c47c8, pconf=0x0, plog=0x0, ptemp=0x0)
     at modperl_callback.c:187
#13 0x40287c9a in modperl_callback_per_dir (idx=6, r=0x8cf8260) at 
modperl_callback.c:218
#14 0x40280682 in modperl_response_handler_run (r=0x8cf8260, finish=1) at 
mod_perl.c:793
#15 0x40280723 in modperl_response_handler (r=0x8cf8260) at mod_perl.c:815
#16 0x8084cb2 in ap_run_handler (r=0x8cf8260) at config.c:194
#17 0x80852e3 in ap_invoke_handler (r=0x8cf8260) at config.c:401
#18 0x8072edf in ap_process_request (r=0x8cf8260) at http_request.c:288
#19 0x806e674 in ap_process_http_connection (c=0x8cf4318) at http_core.c:293
#20 0x808ee3c in ap_run_process_connection (c=0x8cf4318) at connection.c:85
#21 0x808f121 in ap_process_connection (c=0x8cf4318, csd=0x8cf4250) at 
connection.c:211
#22 0x8083752 in child_main (child_num_arg=0) at prefork.c:643
#23 0x808388b in make_child (s=0x80c47c8, slot=0) at prefork.c:737
#24 0x80838f1 in startup_children (number_to_start=1) at prefork.c:755
#25 0x8083c9b in ap_mpm_run (_pconf=0x80c1d70, plog=0x80f9e50, s=0x80c47c8) 
at prefork.c:971
#26 0x8089a37 in main (argc=7, argv=0xbffff864) at main.c:671

error_log:

[Thu Apr 10 09:26:28 2003] [error] server reached MaxClients setting, 
consider raising the MaxClients setting
pv: (TestFilter::in_init_basic::init), len: 33
string PV: [TestFilter::in_init_basic::init]
init was invoked
[Thu Apr 10 09:26:32 2003] [crit] [Thu Apr 10 09:26:32 2003] file 
http_protocol.c, line 1926, assertion "!(((&(bb)->list))->next ==
(struct apr_bucket *)((char *)((&(bb)->list)) - ((long) (((char *) 
(&(((struct apr_bucket*)((void *)0))-> link))) - ((char *) ((void
  *)0))))))" failed
[Thu Apr 10 09:26:34 2003] [info] removed PID file 
/src/modperl-2.0/t/logs/httpd.pid (pid=14377)
[Thu Apr 10 09:26:34 2003] [notice] caught SIGTERM, shutting down
pv: (TestFilter::in_init_basic::init), len: 33
string PV: [TestFilter::in_init_basic::init]




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