perl-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Philippe M. Chiasson" <go...@cpan.org>
Subject Re: [mp1 Patch] Looking for magic in Apache->request
Date Thu, 29 May 2003 03:41:41 GMT
On Thu, 2003-05-29 at 07:13, Stas Bekman wrote:
> Geoffrey Young wrote:
> > 
> >> The problem pointed out by Ken is that it would be nice to be able to
> >> call
> >>
> >> Apache->request($hr);
> >>
> >> So that later call to Apache->request (by other modules) would
> >> return the subclassed object.
> > 
> > 
> > it would not only be nice, request() is documented to behave this way :)
> > 
> >>
> >> Well, the following patch does just that. Seems fine to me but I'd like
> >> to get a few more eyeballs on this one.
> > 
> > 
> > nice work philippe :)
> 
> gozer++
> 
> > the only thing that concerns me here is the stability this may take away 
> > from the 1.0 tree for very little value - although stuff like 
> > Apache::Filter might benefit from this feature, I'm not sure that adding 
> > it now is worth it, since any interested modules are becoming deprecated 
> > in 2.0.
> 
> /me has the same concern. May be keep it in the STATUS file as an available 
> patch. After all there was only one request for this feature. If the patch is 
> available those who want it can try it and if tested enough, can enter 1.29.

Sounds like a good idea to me!

> regarding the patch. why do you keep both IV and SV? Also looks like you mix 
> usages of NULL and Nullsv for SV* values. Otherwise looks good.

Well, on the first item of having both IV and SV, it's quite simple in
an evil way.

My first attempt, after Doug's suggestion in his original comments, was
to simply convert mp_request_rec from an IV to a SV.

But there are problems with that approach. Whenever a user gets a $r,
either thru the first argument of a handle or thru Apache->request, a
new RV is created and blessed on the fly for it. Plus, in more than one
place, the value of mp_request_rec is reset by the mp core in case of
subrequests and so on.

After some major fiddling, I've found that in 99% of the cases,
Apache->request(something) won't be used, so converting a simple IV to a
SV was both overkill and a reference count nightmare.

So the current patch simply keeps using the good old IV most of the
time, but if and when someone starts assigning it thru
Apache->request(), that user-supplied SV gets saved in mp_request_rec_sv
and successive calls to Apache->request will return it.

Also worth noting as a side effect, DESTROY will be called on that
object after the CleanupHandlers have been run.

And yes, I did confuse a NULL with a Nullsv ;-(

As a side note, there are a few places in lib that still call
Apache->request($r) for what seems to me like bogus reasons.

Objections to at least cleaning that stuff up prior to 1.28 ?

# $Id: request_rec_sv.patch,v 1.2 2003/05/29 03:40:55 gozer Exp $

Index: t/net/perl/api.pl
===================================================================
RCS file: /home/cvs/modperl/t/net/perl/api.pl,v
retrieving revision 1.51
diff -u -I$Id -r1.51 api.pl
--- t/net/perl/api.pl	25 May 2003 10:54:08 -0000	1.51
+++ t/net/perl/api.pl	28 May 2003 12:53:52 -0000
@@ -17,7 +17,7 @@
 
 my $is_xs = ($r->uri =~ /_xs/);
 
-my $tests = 81;
+my $tests = 82;
 my $is_win32 = WIN32;
 $tests += 4 unless $is_win32;
 my $test_get_set = Apache->can('set_handlers') && ($tests += 4);
@@ -297,6 +297,11 @@
 }, "My::Req";
 
 test ++$i, $hr->filename;
+
+Apache->request($hr);
+
+test ++$i, ref(Apache->request) eq "My::Req";
+
 delete $hr->{_r};
 my $uri;
 
Index: src/modules/perl/Apache.xs
===================================================================
RCS file: /home/cvs/modperl/src/modules/perl/Apache.xs,v
retrieving revision 1.127
diff -u -I$Id -r1.127 Apache.xs
--- src/modules/perl/Apache.xs	14 Mar 2003 06:05:06 -0000	1.127
+++ src/modules/perl/Apache.xs	28 May 2003 12:53:54 -0000
@@ -1343,15 +1343,17 @@
 #see httpd.h
 #struct request_rec {
 
-void
-request(self, r=NULL)
+SV *
+request(self, r=Nullsv)
     SV *self
-    Apache r
+    SV *r
 
-    PPCODE: 
-    self = self;
-    if(items > 1) perl_request_rec(r);
-    XPUSHs(perl_bless_request_rec(perl_request_rec(NULL)));
+    CODE:
+    if(r) perl_request_rec_sv(r);
+    RETVAL = perl_request_rec_sv(NULL);
+    
+    OUTPUT:
+    RETVAL
 
 #  pool *pool;
 #  conn_rec *connection;
Index: src/modules/perl/mod_perl.c
===================================================================
RCS file: /home/cvs/modperl/src/modules/perl/mod_perl.c,v
retrieving revision 1.146
diff -u -I$Id -r1.146 mod_perl.c
--- src/modules/perl/mod_perl.c	14 Mar 2003 04:45:52 -0000	1.146
+++ src/modules/perl/mod_perl.c	28 May 2003 12:53:54 -0000
@@ -64,6 +64,7 @@
 #endif
 
 static IV mp_request_rec;
+static SV *mp_request_rec_sv = Nullsv;
 static int seqno = 0;
 static int perl_is_running = 0;
 int mod_perl_socketexitoption = 3;
@@ -1141,6 +1142,12 @@
     perl_run_rgy_endav(r->uri);
     per_request_cleanup(r);
 
+    if(mp_request_rec_sv && SvREFCNT(mp_request_rec_sv)) {
+        fprintf(stderr, "Freeing mp_request_rec_sv (refcnt=%d)\n", SvREFCNT(mp_request_rec_sv));
+        SvREFCNT_dec(mp_request_rec_sv);
+    }
+    mp_request_rec_sv = Nullsv;
+
     /* clear %ENV */
     perl_clear_env();
 
@@ -1716,6 +1723,22 @@
     }
     else
 	return (request_rec *)mp_request_rec;
+}
+
+SV *perl_request_rec_sv(SV *r)
+{
+    if(r != Nullsv) {
+        if(mp_request_rec_sv && SvREFCNT(mp_request_rec_sv))
+            SvREFCNT_dec(mp_request_rec_sv);
+        mp_request_rec_sv = SvREFCNT_inc(r);
+        return NULL;
+    }
+    else if(mp_request_rec_sv) {
+        return SvREFCNT_inc(mp_request_rec_sv);
+    }
+    else {
+        return sv_setref_pv(newSV(0), "Apache", perl_request_rec(NULL));
+    }
 }
 
 SV *perl_bless_request_rec(request_rec *r)
Index: lib/Apache/Registry.pm
===================================================================
RCS file: /home/cvs/modperl/lib/Apache/Registry.pm,v
retrieving revision 1.34
diff -u -I$Id -r1.34 Registry.pm
--- lib/Apache/Registry.pm	23 May 2002 04:21:07 -0000	1.34
+++ lib/Apache/Registry.pm	28 May 2003 12:53:54 -0000
@@ -33,13 +33,6 @@
 
 sub handler {
     my $r = shift;
-    if(ref $r) {
-	$r->request($r);
-    }
-    else {
-	#warn "Registry args are: ($r, @_)\n";
-	$r = Apache->request;
-    }
     my $filename = $r->filename;
     #local $0 = $filename; #this core dumps!?
     *0 = \$filename;
Index: lib/Apache/Status.pm
===================================================================
RCS file: /home/cvs/modperl/lib/Apache/Status.pm,v
retrieving revision 1.28
diff -u -I$Id -r1.28 Status.pm
--- lib/Apache/Status.pm	28 Nov 2002 09:42:45 -0000	1.28
+++ lib/Apache/Status.pm	28 May 2003 12:53:54 -0000
@@ -55,7 +55,6 @@
 
 sub handler {
     my($r) = @_;
-    Apache->request($r); #for Apache::CGI
     my $qs = $r->args || "";
     my $sub = "status_$qs";
     no strict 'refs';



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

Mime
View raw message