Stas Bekman wrote:
>Steve Hay wrote:
>
>
>
>>I now get this in the stderr.txt file:
>>
>>SV = PV(0x31f64d0) at 0x31960b8
>> REFCNT = 1
>> FLAGS = (TEMP,POK,pPOK)
>> PV = 0x31f7ed4 "ModPerl::Util::exit: (120000) exit was called at
>>C:/apache2/perl5/site/lib/Apache/Test.pm line 238"\0
>> CUR = 98
>> LEN = 99
>>Attempt to free temp prematurely: SV 0x31960b8, Perl interpreter:
>>0x2b51784 at
>>C:/Temp/bug-reporting-skeleton-mp2/t/response/TestPerl/ithreads.pm line 56.
>>Scalars leaked: 1
>>
>>
>
>Now when the handler returns its status goes through modperl_errsv() which
>has a special case for objects thrown from exit()'s call.
>
>int modperl_errsv(pTHX_ int status, request_rec *r, server_rec *s)
>{
> SV *sv = ERRSV;
> STRLEN n_a;
>
> if (SvTRUE(sv)) {
> if (sv_derived_from(sv, "APR::Error") &&
> SvIVx(sv) == MODPERL_RC_EXIT) {
> /* ModPerl::Util::exit was called */
> return OK;
> }
>
>the actual stringification of the exception object into a string:
>
>"ModPerl::Util::exit: (120000) exit was called at
>C:/apache2/perl5/site/lib/Apache/Test.pm line 238"\0
>
>happens only if something tries to stringify $@ or bool'ify. But this
>stringification is done by a perl code in APR::Error, which overloads:
>
>use overload
> nomethod => \&fatal,
> 'bool' => \&str,
> '==' => \&num,
> '0+' => \&num,
> '""' => \&str;
>
>sub str {
> sprintf "%s: (%d) %s at %s line %d", $_[0]->{func},
> $_[0]->{rc}, APR::Error::strerror($_[0]->{rc}),
> $_[0]->{file}, $_[0]->{line};
>}
>
>and this is what your SV's PV string looks like. So there is no really any
>C code here that could get the refcounting wrong.
>
>may be this is something triggered by overload? what happens if you
>comment out the overload part?
>
>
It is the 'bool' overload that's doing the stringification: presumably
the SvTRUE(sv) call in modperl_errsv()?
If I comment-out the 'bool' overload line then stderr.txt is now empty,
and the skeleton tests pass OK, even with the "use warnings FATAL =>
'all';" line back in.
>(notice that APR::Error is autogenerated, so you may want to modify
>blib/lib/Apache2/APR/Error.pm if you don't want to rebuild things)
>
>alternatively try to add CORE::dump() inside str() above to see who has
>called the stringification function.
>
CORE::dump() doesn't seem to do anything useful in Win32 land :( I
tried commenting-out the "$Carp::CarpInternal{+__PACKAGE__}++;" line in
APR/Error.pm and then adding:
Carp::cluck("str called");
inside str(). This only gives me the following in stderr.txt:
str called at C:/apache2/perl5/site/lib/APR/Error.pm line 60
APR::Error::str('APR::Error=HASH(0x2b2107c)', 'undef', '') called at
-e line 0
str called at C:/apache2/perl5/site/lib/APR/Error.pm line 60
APR::Error::str('APR::Error=HASH(0x2b2107c)', 'undef', '') called at
-e line 0
str called at C:/apache2/perl5/site/lib/APR/Error.pm line 60
APR::Error::str('APR::Error=HASH(0x2b2107c)', 'undef', '') called at
-e line 0
Line 60 is, of course, the Carp::cluck() call that I've inserted
itself. Is there any way to get at what "-e line 0" really is?
I was confused where the SV that's being prematurely free()'d was coming
from, though. I thought the 'bool' override would just fill in the PV
slot of the ERRSV, but it isn't the ERRSV that we're having trouble
free()'ing. The sv_dump() quoted at the start of this mail shows that
the offending SV *only* contains a PV slot, not any of the other stuff
that ERRSV would have, and equally if I sv_dump() ERRSV, even just
*after* the SvTRUE() call, it doesn't seem to have a PV slot.
So it's like the stringified error was put in a brand new SVPV...
Then it hit me (I hope this is right...): it must be the string returned
by the str() subroutine itself! Changing the subroutine from what you
quoted above to this:
sub str {
my $str = sprintf "%s: (%d) %s at %s line %d", $_[0]->{func},
$_[0]->{rc}, APR::Error::strerror($_[0]->{rc}),
$_[0]->{file}, $_[0]->{line};
return $str;
}
i.e. explicitly create a lexical for the $str and then return that,
fixes it!
In this case, the string is definitely a lexical variable and evidently
gets cleaned up exactly as we would expect. In the previous case, I'm
not entirely clear in my mind about exactly what
sub str {
sprintf ...
}
does. Clearly it makes some kind of string and implicitly returns it,
but I'm not sure what the scope of the SV that it made is. Why on earth
it would be a problem for me on Win32 and not for you on Linux, I've no
idea.
I've never seen anything like this before, but then again, I've never
liked the "falling off the end" return style anyway -- I always create
lexicals and explcitily return them.
Anyway, svn @ revision 111817 + the attached patch passes all tests for me!
- Steve
------------------------------------------------
Radan Computational Ltd.
We would like to take this opportunity to wish all our customers, suppliers and
colleagues seasons greetings. We will not be sending corporate greetings
cards this year. Instead, we will be making a donation to charity.
The information contained in this message and any files transmitted with it are
confidential and intended for the addressee(s) only. If you have received this
message in error or there are any problems, please notify the sender
immediately. The unauthorized use, disclosure, copying or alteration of this
message is strictly forbidden. Note that any views or opinions presented in
this email are solely those of the author and do not necessarily represent
those of Radan Computational Ltd. The recipient(s) of this message should
check it and any attached files for viruses: Radan Computational will accept no
liability for any damage caused by any virus transmitted by this email.
Index: xs/APR/Error/Error_pm
===================================================================
--- xs/APR/Error/Error_pm (revision 111817)
+++ xs/APR/Error/Error_pm (working copy)
@@ -30,9 +30,10 @@
# - the filename and line number are needed because perl doesn't
# provide that info when exception objects are involved
sub str {
- sprintf "%s: (%d) %s at %s line %d", $_[0]->{func},
+ my $str = sprintf "%s: (%d) %s at %s line %d", $_[0]->{func},
$_[0]->{rc}, APR::Error::strerror($_[0]->{rc}),
$_[0]->{file}, $_[0]->{line};
+ return $str;
}
sub num { $_[0]->{rc} }
Index: t/perl/ithreads.t
===================================================================
--- t/perl/ithreads.t (revision 111817)
+++ t/perl/ithreads.t (working copy)
@@ -6,15 +6,11 @@
use Apache::Test;
use Apache::TestRequest 'GET_BODY_ASSERT';
-# XXX: at the moment this test randomly fails, making it impossible to
-# debug, so skip it for now and get back to work on it later.
-
# perl < 5.6.0 fails to compile code with 'shared' attributes, so we must skip
# it here.
-#unless ($] >= 5.008001 && $Config{useithreads}) {
-# plan tests => 1, need
-# {"perl 5.8.1 or higher w/ithreads enabled is required" => 0};
-#}
-plan tests => 1, under_construction;
+unless ($] >= 5.008001 && $Config{useithreads}) {
+ plan tests => 1, need
+ {"perl 5.8.1 or higher w/ithreads enabled is required" => 0};
+}
print GET_BODY_ASSERT "/TestPerl__ithreads";
---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
