You are viewing a plain text version of this content. The canonical link for it is here.
Posted to modperl@perl.apache.org by Stas Bekman <st...@stason.org> on 2005/04/07 21:42:33 UTC

Re: [PATCH] Re: [BUG] Bad refcounting in Apache->push_handlers()

Dominique Quatravaux wrote:
>>Any chance you could add a test to the modperl2 test suite that I can 
>>reproduce the problem with?
> 
> 
> I finally got around to doing that, sorry for the delay. Here is a new
> test case, with inspiration from TestHooks::inlined_handlers.
> 
> Warning: since Apache->server->push_handlers() needs to operate at the
> global server-config level, once this patch is applied, the *entire*
> test suite mostly stops working :-] However, I am confident that once
> the bug is gone this patch will no longer be intrusive, because when I
> un-inline the sub (there is a commented-out line to that effect in the
> test case file), the test suite passes correctly as a whole.

Dominique, your test case had a bug, fixed as following:

- sub { test_waz_zere });
+ sub { &test_waz_zere });

or  sub { test_waz_zere(@_) });

also I did some updates for the recent changes in mp2 API, the whole test is at the end.

Now with threaded perl 5.8.6 everything works fine, but:
t/directive/perlrequire fails with: can't find ANONSUB top entry (get)

with non-threaded 5.6.1 a bunch of tests fail with:
Undefined subroutine &main::ÿÿÿÿÿÿÿÿÿÿÿÿÿ called, <DATA> line 462.

so I'll be looking at those cases.

Thanks for the test case!

package TestHooks::inlined_handlers_server_wide;

# this test exercises Apache handlers as anonymous subs, installed
# using Apache->server->push_handlers() at mod_perl initialization
# time.
# Previously there was a refcounting bug there, similar to the
# one described in inlined_handlers.pm

use strict;
use warnings FATAL => 'all';

use Apache2::RequestIO ();
use Apache2::ServerUtil ();
use Apache2::Const -compile => qw(OK DECLINED);

our $answer = "not ok 1\n";

sub test_waz_zere {
    my ($r) = @_;
    $r->notes->set("test_waz_zere", 1);
    Apache2::Const::DECLINED;
}

BEGIN { if ($ENV{MOD_PERL}) {
## This works (non-inlined handler):
#    Apache2->server->push_handlers(PerlFixupHandler => \&test_waz_zere );
## This did not work as of Subversion r159573:
    Apache2::ServerUtil->server->push_handlers(PerlFixupHandler => 
                                               sub { test_waz_zere(@_) });
} }

sub handler {
    my $r = shift;

    $r->print("1..1\n");
    $r->print($r->notes->get("test_waz_zere") ? "ok 1\n" :
             "not ok 1\n");
    Apache2::Const::OK;
}

1;
__DATA__
<NoAutoConfig>
    PerlModule TestHooks::inlined_handlers_server_wide
  <Location /TestHooks__inlined_handlers_server_wide>
      SetHandler modperl
      PerlResponseHandler TestHooks::inlined_handlers_server_wide
  </Location>
</NoAutoConfig>


-- 
__________________________________________________________________
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] Re: [BUG] Bad refcounting in Apache->push_handlers()

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

Still need to check this one:

> Now with threaded perl 5.8.6 everything works fine, but:
> t/directive/perlrequire fails with: can't find ANONSUB top entry (get)

The patch below fixes the non-threaded case.
 
> with non-threaded 5.6.1 a bunch of tests fail with:
> Undefined subroutine &main::ÿÿÿÿÿÿÿÿÿÿÿÿÿ called, <DATA> line 462.

Index: src/modules/perl/modperl_handler.c
===================================================================
--- src/modules/perl/modperl_handler.c  (revision 160427)
+++ src/modules/perl/modperl_handler.c  (working copy)
@@ -49,6 +49,7 @@
  *    PerlTransHandler 'sub { ... }'
  * B) run-time perl code
  *    $r->push_handlers(PerlTransHandler => sub { .... });
+ *    $s->push_handlers(PerlTransHandler => sub { .... });
  *
  * In the case of non-threaded perl, we just compile A or grab B and
  * store it in the mod_perl struct and call it when it's used. No
@@ -169,7 +170,7 @@
 #else
     /* it's safe to cache and later use the cv, since the same perl
      * interpeter is always used */
-    handler->cv   = cv;
+    handler->cv   = SvREFCNT_inc((SV*)cv);
     handler->name = NULL;

     MP_TRACE_h(MP_FUNC, "[%s] new cached cv anon handler\n",


-- 
__________________________________________________________________
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] Re: [BUG] Bad refcounting in Apache->push_handlers()

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

Still need to check this one:

> Now with threaded perl 5.8.6 everything works fine, but:
> t/directive/perlrequire fails with: can't find ANONSUB top entry (get)

The patch below fixes the non-threaded case.
 
> with non-threaded 5.6.1 a bunch of tests fail with:
> Undefined subroutine &main::ÿÿÿÿÿÿÿÿÿÿÿÿÿ called, <DATA> line 462.

Index: src/modules/perl/modperl_handler.c
===================================================================
--- src/modules/perl/modperl_handler.c  (revision 160427)
+++ src/modules/perl/modperl_handler.c  (working copy)
@@ -49,6 +49,7 @@
  *    PerlTransHandler 'sub { ... }'
  * B) run-time perl code
  *    $r->push_handlers(PerlTransHandler => sub { .... });
+ *    $s->push_handlers(PerlTransHandler => sub { .... });
  *
  * In the case of non-threaded perl, we just compile A or grab B and
  * store it in the mod_perl struct and call it when it's used. No
@@ -169,7 +170,7 @@
 #else
     /* it's safe to cache and later use the cv, since the same perl
      * interpeter is always used */
-    handler->cv   = cv;
+    handler->cv   = SvREFCNT_inc((SV*)cv);
     handler->name = NULL;

     MP_TRACE_h(MP_FUNC, "[%s] new cached cv anon handler\n",


-- 
__________________________________________________________________
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