RE: $r-get_handlers bug/oversight?
-Original Message-
From: Doug MacEachern [mailto:[EMAIL PROTECTED]]
Sent: Monday, August 21, 2000 6:41 PM
To: Geoffrey Young
Cc: '[EMAIL PROTECTED]'
Subject: RE: $r-get_handlers bug/oversight?
On Wed, 16 Aug 2000, Geoffrey Young wrote:
ack... so the alias only goes one way? I guess it makes
sense that we
can't know at run time what the Init handler stands for,
but how come
get_handlers('PerlInitHandler') comes up blank? Isn't it
just a table
entry?
it's not in the get/set handler lookup table. just use
PostReadRequest/HeaderParser for now, we'll see about making
Init do the
right thing with get/set handlers later.
fair enough :)
the patch looks great - thanks for spending the time...
--Geoff
RE: $r-get_handlers bug/oversight?
On Wed, 16 Aug 2000, Geoffrey Young wrote:
ack... so the alias only goes one way? I guess it makes sense that we
can't know at run time what the Init handler stands for, but how come
get_handlers('PerlInitHandler') comes up blank? Isn't it just a table
entry?
it's not in the get/set handler lookup table. just use
PostReadRequest/HeaderParser for now, we'll see about making Init do the
right thing with get/set handlers later.
well, it got the handler ok, but I couldn't set it properly:
#!/usr/bin/perl
my $r = shift;
$r-set_handlers(PerlCleanupHandler = [\cleanup]);
$r-send_http_header('text/plain');
print "done";
sub cleanup {
warn "hi";
}
is a no go. same with using ['My::Cleanup'] as the arg...
ok, fixed, problem described in new Apache.xs comment:
/* since register_cleanups are fifo, and the already registered
* mod_perl_end_cleanup() runs PerlCleanupHandlers, PerlCleanupHandler
* needs to maintain the refcnt itself
*/
I also noticed that the patch didn't fix the get_handlers() coderef bug:
#!/usr/bin/perl
my $r = shift;
$r-push_handlers(PerlCleanupHandler = sub { warn "hi"; });
#my $handlers = $r-get_handlers('PerlCleanupHandler');
$r-send_http_header('text/plain');
print "done";
uncomment the get_handlers() line and the cleanup handler never runs and you
get "Attempt to free unreferenced scalar"
i see the problem, perl_handler_merge_avs() did not increment the
reference counts during av_push(). below is the current patch against cvs
which fixes both problems. thanks for testing geoff!
Index: src/modules/perl/Apache.xs
===
RCS file: /home/cvs/modperl/src/modules/perl/Apache.xs,v
retrieving revision 1.103
diff -u -r1.103 Apache.xs
--- src/modules/perl/Apache.xs 2000/08/15 19:36:32 1.103
+++ src/modules/perl/Apache.xs 2000/08/21 22:33:32
@@ -73,12 +73,6 @@
void (*set_func) (void *, void *, SV *);
} perl_handler_table;
-typedef struct {
-I32 fill;
-AV *av;
-AV **ptr;
-} perl_save_av;
-
static void set_handler_dir (perl_handler_table *tab, request_rec *r, SV *sv);
static void set_handler_srv (perl_handler_table *tab, request_rec *r, SV *sv);
@@ -101,77 +95,78 @@
{HandlerDirEntry("PerlFixupHandler", PerlFixupHandler)},
{HandlerDirEntry("PerlHandler", PerlHandler)},
{HandlerDirEntry("PerlLogHandler", PerlLogHandler)},
+{HandlerDirEntry("PerlCleanupHandler", PerlCleanupHandler)},
{ FALSE, NULL }
};
-static void perl_restore_av(void *data)
-{
-perl_save_av *save_av = (perl_save_av *)data;
-
-if(save_av-fill != DONE) {
- AvFILLp(*save_av-ptr) = save_av-fill;
-}
-else if(save_av-av != Nullav) {
- *save_av-ptr = save_av-av;
-}
-}
-
static void perl_handler_merge_avs(char *hook, AV **dest)
{
int i = 0;
HV *hv = perl_get_hv("Apache::PerlStackedHandlers", FALSE);
SV **svp = hv_fetch(hv, hook, strlen(hook), FALSE);
AV *base;
-
+
if(!(svp SvROK(*svp)))
return;
base = (AV*)SvRV(*svp);
for(i=0; i=AvFILL(base); i++) {
SV *sv = *av_fetch(base, i, FALSE);
- av_push(*dest, sv);
+ av_push(*dest, SvREFCNT_inc(sv));
}
}
+#define avptr_from_offset(ptr, tab) \
+(AV **)((char *)ptr + (int)(long)tab-offset)
+
static void set_handler_base(void *ptr, perl_handler_table *tab, pool *p, SV *sv)
{
-AV **av = (AV **)((char *)ptr + (int)(long)tab-offset);
+int do_register_cleanup = 0;
+AV **av = avptr_from_offset(ptr, tab);
-perl_save_av *save_av =
- (perl_save_av *)palloc(p, sizeof(perl_save_av));
-
-save_av-fill = DONE;
-save_av-av = Nullav;
-
-if((sv == sv_undef) || (SvIOK(sv) SvIV(sv) == DONE)) {
- if(AvTRUE(*av)) {
- save_av-fill = AvFILL(*av);
- AvFILLp(*av) = -1;
- }
-}
-else if(SvROK(sv) SvTYPE(SvRV(sv)) == SVt_PVAV) {
- if(AvTRUE(*av))
- save_av-av = av_copy_array(*av);
- *av = (AV*)SvRV(sv);
- ++SvREFCNT(*av);
+if ((sv == sv_undef) || (SvIOK(sv) SvIV(sv) == DONE)) {
+if (!*av) {
+do_register_cleanup = 1;
+}
+if (*av SvREFCNT(*av)) {
+SvREFCNT_dec(*av);
+}
+*av = newAV();
+}
+else if (SvROK(sv) SvTYPE(SvRV(sv)) == SVt_PVAV) {
+*av = (AV*)SvRV(sv);
+++SvREFCNT(*av);
+do_register_cleanup = 1;
}
else {
- croak("Can't set_handler with that value");
+croak("Can't set_handler with that value");
+}
+
+/* since register_cleanups are fifo, and the already registered
+ * mod_perl_end_cleanup() runs PerlCleanupHandlers, PerlCleanupHandler
+ * needs to maintain the refcnt itself
+ */
+if (do_register_cleanup strNE(tab-name, "PerlCleanupHandler")) {
+register_cleanup(p, (void*)*av, mod_perl_cleanup_av, mod_perl_noop);
}
-save_av-ptr = av;
-register_cleanup(p,
RE: $r-get_handlers bug/oversight?
-Original Message-
From: Doug MacEachern [mailto:[EMAIL PROTECTED]]
Sent: Tuesday, August 15, 2000 11:22 PM
To: Geoffrey Young
Cc: '[EMAIL PROTECTED]'
Subject: Re: $r-get_handlers bug/oversight?
On Tue, 25 Apr 2000, Geoffrey Young wrote:
Hi all...
I've noticed that get_handlers() will return the
enabled handlers
for a PerlPostReadRequestHandler, but not when it is specified as a
PerlInitHandler (either by calling
$r-get_handlers('PerlPostReadRequestHandler') or
$r-get_handlers('PerlInitHandler'). It is the same with
PerlHeaderParserHandler. An oversight?
PerlInitHandler is just an alias for PerlPostReadRequestHandler and
PerlHeaderParserHandler. mod_perl can only know at config-time if
InitHandler should be alias to PostReadRequest or
HeaderParser, i don't
think get_handlers() can figure out which you mean at request time.
ack... so the alias only goes one way? I guess it makes sense that we
can't know at run time what the Init handler stands for, but how come
get_handlers('PerlInitHandler') comes up blank? Isn't it just a table
entry?
Also, I can't get anything for PerlCleanupHandlers, which kinda
makes sense, since Cleanup isn't really a phase, per se (at
least according
to the book). Does it make sense to add this to
get_handlers() as well?
the get_handlers() patch posted earlier should enable get/set of
PerlCleanupHandlers.
well, it got the handler ok, but I couldn't set it properly:
#!/usr/bin/perl
my $r = shift;
$r-set_handlers(PerlCleanupHandler = [\cleanup]);
$r-send_http_header('text/plain');
print "done";
sub cleanup {
warn "hi";
}
is a no go. same with using ['My::Cleanup'] as the arg...
I also noticed that the patch didn't fix the get_handlers() coderef bug:
#!/usr/bin/perl
my $r = shift;
$r-push_handlers(PerlCleanupHandler = sub { warn "hi"; });
#my $handlers = $r-get_handlers('PerlCleanupHandler');
$r-send_http_header('text/plain');
print "done";
uncomment the get_handlers() line and the cleanup handler never runs and you
get "Attempt to free unreferenced scalar"
if you have some tuits :)
--Geoff
Re: $r-get_handlers bug/oversight?
On Tue, 25 Apr 2000, Geoffrey Young wrote:
Hi all...
I've noticed that get_handlers() will return the enabled handlers
for a PerlPostReadRequestHandler, but not when it is specified as a
PerlInitHandler (either by calling
$r-get_handlers('PerlPostReadRequestHandler') or
$r-get_handlers('PerlInitHandler'). It is the same with
PerlHeaderParserHandler. An oversight?
PerlInitHandler is just an alias for PerlPostReadRequestHandler and
PerlHeaderParserHandler. mod_perl can only know at config-time if
InitHandler should be alias to PostReadRequest or HeaderParser, i don't
think get_handlers() can figure out which you mean at request time.
Also, I can't get anything for PerlCleanupHandlers, which kinda
makes sense, since Cleanup isn't really a phase, per se (at least according
to the book). Does it make sense to add this to get_handlers() as well?
the get_handlers() patch posted earlier should enable get/set of
PerlCleanupHandlers.
RE: $r-get_handlers bug/oversight? (possible fix)
On Tue, 2 May 2000, Geoffrey Young wrote: ok, for anyone who is interested, I seem to have fixed the problem (maybe)... here's a patch for Apache.xs from yesterday's cvs (and I didn't see any commits since then...) --- Apache.xs.old Tue May 2 14:25:09 2000 +++ Apache.xs Tue May 2 14:26:19 2000 @@ -220,7 +220,7 @@ perl_handler_merge_avs(hook, avcopy); -return newRV_noinc((SV*)avcopy); +return newRV_inc((SV*)avcopy); } now, the implications of this are way over my head, but it seems to work with the test case below just fine (and my other code with which I initially caught the problem). anyway... hmm, that could result in a leak. i'll look into this soon, thanks for the details and patch!
RE: $r-get_handlers bug/oversight? (possible fix)
ok, for anyone who is interested, I seem to have fixed the problem
(maybe)...
here's a patch for Apache.xs from yesterday's cvs (and I didn't see any
commits since then...)
--- Apache.xs.old Tue May 2 14:25:09 2000
+++ Apache.xs Tue May 2 14:26:19 2000
@@ -220,7 +220,7 @@
perl_handler_merge_avs(hook, avcopy);
-return newRV_noinc((SV*)avcopy);
+return newRV_inc((SV*)avcopy);
}
now, the implications of this are way over my head, but it seems to work
with the test case below just fine (and my other code with which I initially
caught the problem). anyway...
HTH
--Geoff
-Original Message-
From: Geoffrey Young [mailto:[EMAIL PROTECTED]]
Sent: Monday, May 01, 2000 4:32 PM
To: '[EMAIL PROTECTED]'
Cc: '[EMAIL PROTECTED]'
Subject: RE: $r-get_handlers bug/oversight?
hi again...
I'm having lots of problems with the get_handlers method...
the following is
reproducible for me under 1.22, 1.23 and the latest cvs using
1.3.12...
---
#!/usr/bin/perl
my $r = shift;
my $list;
my @array = qw('test' 'array');
$r-pnotes(TEST = \@array);
$r-push_handlers(PerlLogHandler = sub {
my $pnotes = $r-pnotes;
foreach my $key (sort keys %$pnotes) {
warn "this is the key $key";
};
});
#$list = $r-get_handlers('PerlLogHandler');
$r-send_http_header('text/plain');
foreach my $key (@$list) {
print "$key\n";
}
print "done!";
---
running as is prints the pnotes keys.
uncommenting the get_handlers method gives:
Attempt to free unreferenced scalar.
and no other output, yet a code reference is visible in the browser...
The other thing is that this only seems to be an issue for
code references -
if I push My::Logger instead of a subroutine, all is fine...
Am I using push_handlers incorrectly, or is get_handlers
mucked up (or can
nobody reproduce this)?
--Geoff
On Tue, 25 Apr 2000, Geoffrey Young wrote:
Hi all...
I've noticed that get_handlers() will return the
enabled handlers
for a PerlPostReadRequestHandler, but not when it is specified as a
PerlInitHandler (either by calling
$r-get_handlers('PerlPostReadRequestHandler') or
$r-get_handlers('PerlInitHandler'). It is the same with
PerlHeaderParserHandler. An oversight?
Also, I can't get anything for PerlCleanupHandlers,
which kinda
makes sense, since Cleanup isn't really a phase, per se (at least
according
to the book). Does it make sense to add this to
get_handlers() as well?
oversight. neither CleanupHandler nor InitHandlers is in the
handler_table in Apache.xs. probably because those
directives were added
after get/set handlers was implemented, and the table was
never updated.
i'll see about fixing that.
RE: $r-get_handlers bug/oversight?
hi again...
I'm having lots of problems with the get_handlers method... the following is
reproducible for me under 1.22, 1.23 and the latest cvs using 1.3.12...
---
#!/usr/bin/perl
my $r = shift;
my $list;
my @array = qw('test' 'array');
$r-pnotes(TEST = \@array);
$r-push_handlers(PerlLogHandler = sub {
my $pnotes = $r-pnotes;
foreach my $key (sort keys %$pnotes) {
warn "this is the key $key";
};
});
#$list = $r-get_handlers('PerlLogHandler');
$r-send_http_header('text/plain');
foreach my $key (@$list) {
print "$key\n";
}
print "done!";
---
running as is prints the pnotes keys.
uncommenting the get_handlers method gives:
Attempt to free unreferenced scalar.
and no other output, yet a code reference is visible in the browser...
The other thing is that this only seems to be an issue for code references -
if I push My::Logger instead of a subroutine, all is fine...
Am I using push_handlers incorrectly, or is get_handlers mucked up (or can
nobody reproduce this)?
--Geoff
On Tue, 25 Apr 2000, Geoffrey Young wrote:
Hi all...
I've noticed that get_handlers() will return the enabled handlers
for a PerlPostReadRequestHandler, but not when it is specified as a
PerlInitHandler (either by calling
$r-get_handlers('PerlPostReadRequestHandler') or
$r-get_handlers('PerlInitHandler'). It is the same with
PerlHeaderParserHandler. An oversight?
Also, I can't get anything for PerlCleanupHandlers, which kinda
makes sense, since Cleanup isn't really a phase, per se (at least
according
to the book). Does it make sense to add this to get_handlers() as well?
oversight. neither CleanupHandler nor InitHandlers is in the
handler_table in Apache.xs. probably because those directives were added
after get/set handlers was implemented, and the table was never updated.
i'll see about fixing that.
Re: $r-get_handlers bug/oversight?
On Tue, 25 Apr 2000, Geoffrey Young wrote:
Hi all...
I've noticed that get_handlers() will return the enabled handlers
for a PerlPostReadRequestHandler, but not when it is specified as a
PerlInitHandler (either by calling
$r-get_handlers('PerlPostReadRequestHandler') or
$r-get_handlers('PerlInitHandler'). It is the same with
PerlHeaderParserHandler. An oversight?
Also, I can't get anything for PerlCleanupHandlers, which kinda
makes sense, since Cleanup isn't really a phase, per se (at least according
to the book). Does it make sense to add this to get_handlers() as well?
oversight. neither CleanupHandler nor InitHandlers is in the
handler_table in Apache.xs. probably because those directives were added
after get/set handlers was implemented, and the table was never updated.
i'll see about fixing that.
