You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@perl.apache.org by Torsten Foertsch <to...@gmx.net> on 2007/10/11 15:18:15 UTC
possible pnotes refcounting bug ?
Hi,
this is a snippet from modperl_util.c:modperl_pnotes()
if (key) {
STRLEN len;
char *k = SvPV(key, len);
if (val) {
retval = *hv_store(*pnotes, k, len, SvREFCNT_inc(val), 0);
}
else if (hv_exists(*pnotes, k, len)) {
retval = *hv_fetch(*pnotes, k, len, FALSE);
}
}
else {
retval = newRV_inc((SV *)*pnotes);
}
return retval ? SvREFCNT_inc(retval) : &PL_sv_undef;
I am wondering whether the REFCNT is always right. *pnotes is a HV. If the
function is called without a key argument the else branch newRV_inc
increments the REFCNT of the HV, right? Then the return statement in the last
line increments it again? Am I wrong?
Torsten
Re: possible pnotes refcounting bug ?
Posted by Geoffrey Young <ge...@modperlcookbook.org>.
Torsten Foertsch wrote:
> Hi,
>
> this is a snippet from modperl_util.c:modperl_pnotes()
>
> if (key) {
> STRLEN len;
> char *k = SvPV(key, len);
>
> if (val) {
> retval = *hv_store(*pnotes, k, len, SvREFCNT_inc(val), 0);
> }
> else if (hv_exists(*pnotes, k, len)) {
> retval = *hv_fetch(*pnotes, k, len, FALSE);
> }
> }
> else {
> retval = newRV_inc((SV *)*pnotes);
> }
>
> return retval ? SvREFCNT_inc(retval) : &PL_sv_undef;
>
> I am wondering whether the REFCNT is always right. *pnotes is a HV. If the
> function is called without a key argument the else branch newRV_inc
> increments the REFCNT of the HV, right? Then the return statement in the last
> line increments it again? Am I wrong?
philippe is better at this stuff than I am, but I suspect we want to
falsely increment the refcount by (at least) one so that when the retval
goes out of local scope in some handler the value doesn't get cleaned up
by perl and instead waits around for us to manually clean it up at the
end of the request.
--Geoff
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org
Re: possible pnotes refcounting bug ?
Posted by Geoffrey Young <ge...@modperlcookbook.org>.
Torsten Foertsch wrote:
> On Thursday 11 October 2007 15:18, Torsten Foertsch wrote:
>> if (key) {
>> STRLEN len;
>> char *k = SvPV(key, len);
>>
>> if (val) {
>> retval = *hv_store(*pnotes, k, len, SvREFCNT_inc(val), 0);
>> }
>> else if (hv_exists(*pnotes, k, len)) {
>> retval = *hv_fetch(*pnotes, k, len, FALSE);
>> }
>> }
>> else {
>> retval = newRV_inc((SV *)*pnotes);
>> }
>>
>> return retval ? SvREFCNT_inc(retval) : &PL_sv_undef;
>>
>> I am wondering whether the REFCNT is always right. *pnotes is a HV. If the
>> function is called without a key argument the else branch newRV_inc
>> increments the REFCNT of the HV, right? Then the return statement in the
>> last line increments it again? Am I wrong?
>
> I think it should rather read:
>
> if (key) {
> STRLEN len;
> char *k = SvPV(key, len);
>
> if (val) {
> retval = *hv_store(*pnotes, k, len, SvREFCNT_inc(val), 0);
> }
> else if (hv_exists(*pnotes, k, len)) {
> retval = *hv_fetch(*pnotes, k, len, FALSE);
> }
>
> return retval ? SvREFCNT_inc(retval) : &PL_sv_undef;
> }
> else {
> return newRV_inc((SV *)*pnotes);
> }
again, I'll defer to philippe :)
but it seems to me that gives
my $pnotes = $r->pnotes();
a refcount of one. then if someone
$pnotes->($key => $val);
$pnotes still has a refcount of one, so when the handler exits perl will
clean it up and the next handler to ask for pnotes() won't have the new
value. adding one to the count means that the internal representation
won't get cleaned up by perl behind the scenes and muck with what we
expect pnotes to do.
but, in truth, it's been ages since I worked closely with this stuff,
and at my age the mind starts to go rather quickly, so I could be
completely off base :)
--Geoff
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org
Re: possible pnotes refcounting bug ?
Posted by Torsten Foertsch <to...@gmx.net>.
On Thursday 11 October 2007 15:18, Torsten Foertsch wrote:
> if (key) {
> STRLEN len;
> char *k = SvPV(key, len);
>
> if (val) {
> retval = *hv_store(*pnotes, k, len, SvREFCNT_inc(val), 0);
> }
> else if (hv_exists(*pnotes, k, len)) {
> retval = *hv_fetch(*pnotes, k, len, FALSE);
> }
> }
> else {
> retval = newRV_inc((SV *)*pnotes);
> }
>
> return retval ? SvREFCNT_inc(retval) : &PL_sv_undef;
>
> I am wondering whether the REFCNT is always right. *pnotes is a HV. If the
> function is called without a key argument the else branch newRV_inc
> increments the REFCNT of the HV, right? Then the return statement in the
> last line increments it again? Am I wrong?
I think it should rather read:
if (key) {
STRLEN len;
char *k = SvPV(key, len);
if (val) {
retval = *hv_store(*pnotes, k, len, SvREFCNT_inc(val), 0);
}
else if (hv_exists(*pnotes, k, len)) {
retval = *hv_fetch(*pnotes, k, len, FALSE);
}
return retval ? SvREFCNT_inc(retval) : &PL_sv_undef;
}
else {
return newRV_inc((SV *)*pnotes);
}
Torsten
Re: possible pnotes refcounting bug ?
Posted by "Philippe M. Chiasson" <go...@ectoplasm.org>.
Torsten Foertsch wrote:
> On Thursday 11 October 2007 15:18, Torsten Foertsch wrote:
>> I am wondering whether the REFCNT is always right. *pnotes is a HV. If the
>> function is called without a key argument the else branch newRV_inc
>> increments the REFCNT of the HV, right? Then the return statement in the
>> last line increments it again? Am I wrong?
Yup, your typical reference counting bug...
> Here is a test that shows what I mean. Under 2.0.3 I get this:
>
> Failed Test Stat Wstat Total Fail List of Failed
> -------------------------------------------------------------------------------
> t/modperl/pnotes2.t 12 8 2 4-6 8 10-12
>
> The test saves an object in pnotes that on DESTROY prints a message to the
> error_log. Further a CleanupHandler is installed to check if the pnotes are
> destroyed after that phase.
>
> In each failing test pnotes are accessed at least once without arguments. The
> stored object is not destroyed because the REFCNT of the HV is too big.
Fixed in rev 584380!
> ->pnotes($key=>$value) works but has other drawbacks. (my $x=1;
> $r->pnotes(x=>$x); undef $x; # undefs also $r->pnotes->{x})
Yes, and I can't find it in the archives right now, but that was a feature
of sorts. mod_perl-1.x used to do it like that, and it was decided to
continue doing the same thing. Not so obvious to the casual user, but
I believe it's even documented somewhere.
------------------------------------------------------------------------
Philippe M. Chiasson GPG: F9BFE0C2480E7680 1AE53631CB32A107 88C3A5A5
http://gozer.ectoplasm.org/ m/gozer\@(apache|cpan|ectoplasm)\.org/
Re: possible pnotes refcounting bug ?
Posted by Torsten Foertsch <to...@gmx.net>.
On Thursday 11 October 2007 15:18, Torsten Foertsch wrote:
> I am wondering whether the REFCNT is always right. *pnotes is a HV. If the
> function is called without a key argument the else branch newRV_inc
> increments the REFCNT of the HV, right? Then the return statement in the
> last line increments it again? Am I wrong?
Here is a test that shows what I mean. Under 2.0.3 I get this:
Failed Test Stat Wstat Total Fail List of Failed
-------------------------------------------------------------------------------
t/modperl/pnotes2.t 12 8 2 4-6 8 10-12
The test saves an object in pnotes that on DESTROY prints a message to the
error_log. Further a CleanupHandler is installed to check if the pnotes are
destroyed after that phase.
In each failing test pnotes are accessed at least once without arguments. The
stored object is not destroyed because the REFCNT of the HV is too big.
->pnotes($key=>$value) works but has other drawbacks. (my $x=1;
$r->pnotes(x=>$x); undef $x; # undefs also $r->pnotes->{x})
Torsten