Login | Register For Free | Help
Search for: (Advanced)

Mailing List Archive: ModPerl: Dev

[Fwd: [rt.cpan.org #30061] $r->pnotes stores aliases not copies so is subject to action at a distance]

 

 

ModPerl dev RSS feed   Index | Next | Previous | View Threaded


geoff at modperlcookbook

Oct 17, 2007, 8:30 AM

Post #1 of 5 (1618 views)
Permalink
[Fwd: [rt.cpan.org #30061] $r->pnotes stores aliases not copies so is subject to action at a distance]

heh, didn't this just come up?

:)

gozer: I did find the documentation, and I posted it to the rt ticket.

I guess we don't have a historical way of managing rt tickets, but I'm
tempted to resolve this one as "rejected" (though the thought of
rejecting tim isn't a pleasant one ;)

--Geoff

-------- Original Message --------
Subject: [rt.cpan.org #30061] $r->pnotes stores aliases not copies so is
subject to action at a distance
Date: Wed, 17 Oct 2007 10:58:22 -0400
From: Tim_Bunce via RT <bug-mod_perl [at] rt>
Reply-To: bug-mod_perl [at] rt
To: undisclosed-recipients:;
References: <RT-Ticket-30061 [at] rt>


Wed Oct 17 10:57:58 2007: Request 30061 was acted upon.
Transaction: Ticket created by TIMB
Queue: mod_perl
Subject: $r->pnotes stores aliases not copies so is subject to
action at a
distance
Broken in: (no value)
Severity: Important
Owner: Nobody
Requestors: TIMB [at] cpan
Status: new
Ticket <URL: http://rt.cpan.org/Ticket/Display.html?id=30061 >


This code:

$x = 42;
$r->pnotes( 'foo', $x );
++$x;
warn $r->pnotes( 'foo', $x );

should print the value that was stored, 42, but it actually prints 43!

The value stored has been modified by action at a distance. *Not good*

This can cause some very hard to debug problems, where values stored in
pnotes change for
no readily apparent reason. I know, I've spent way too long doing just
that before I identified
this as the cause.

A workaround for affected code is to assign to a temporary variable so
the alias taken by
pnotes isn't an alias for the orginal value:

$r->pnotes( 'foo', my $tmp = $x )

The minimal fix is to change:

hv_store(cfg->pnotes, key, len, SvREFCNT_inc(val), FALSE);

in pnotes in src/modules/perl/Apache.xs to something more like

SV **svp = hv_fetch(cfg->pnotes, key, len, TRUE);
sv_setsv(*svp, val);

with a little more work you could eliminate the earlier hv_exists and
hv_fetch by doing

SV **svp = hv_fetch(cfg->pnotes, key, len, (val) ? TRUE : FALSE);

and working with svp.

Tim Bunce.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe [at] perl
For additional commands, e-mail: dev-help [at] perl


gozer at ectoplasm

Oct 17, 2007, 9:10 AM

Post #2 of 5 (1518 views)
Permalink
Re: [Fwd: [rt.cpan.org #30061] $r->pnotes stores aliases not copies so is subject to action at a distance] [In reply to]

Geoffrey Young wrote:
> heh, didn't this just come up?

It sure did!

> gozer: I did find the documentation, and I posted it to the rt ticket.

I knew it existed somewhere, thanks.

> I guess we don't have a historical way of managing rt tickets, but I'm
> tempted to resolve this one as "rejected" (though the thought of
> rejecting tim isn't a pleasant one ;)

Yeah, I know. If you ask me, this pnotes behaviour is wrong, but we chose
to keep it around for historical reasons. It's not a bug if it's documented ;-)

------------------------------------------------------------------------
Philippe M. Chiasson GPG: F9BFE0C2480E7680 1AE53631CB32A107 88C3A5A5
http://gozer.ectoplasm.org/ m/gozer\@(apache|cpan|ectoplasm)\.org/
Attachments: signature.asc (0.24 KB)


torsten.foertsch at gmx

Oct 17, 2007, 10:32 AM

Post #3 of 5 (1504 views)
Permalink
Re: [Fwd: [rt.cpan.org #30061] $r->pnotes stores aliases not copies so is subject to action at a distance] [In reply to]

On Wednesday 17 October 2007 18:10, Philippe M. Chiasson wrote:
> Yeah, I know. If you ask me, this pnotes behaviour is wrong, but we chose
> to keep it around for historical reasons. It's not a bug if it's documented

It is a bug. If it is documented it is a documented bug. I was hit by it once
and spent a day or so to find it, really nasty. I cannot agree with
the "historical" reason. If something is wrong it it wrong and it doesn't
matter how long it has been wrong before.

For many thousand years the Earth has been flat like a pizza. But now we know
it is a sphere. So the pizza-like Earth was a very long standing bug.

Torsten


geoff at modperlcookbook

Oct 17, 2007, 10:50 AM

Post #4 of 5 (1519 views)
Permalink
Re: [Fwd: [rt.cpan.org #30061] $r->pnotes stores aliases not copies so is subject to action at a distance] [In reply to]

Torsten Foertsch wrote:
> On Wednesday 17 October 2007 18:10, Philippe M. Chiasson wrote:
>> Yeah, I know. If you ask me, this pnotes behaviour is wrong, but we chose
>> to keep it around for historical reasons. It's not a bug if it's documented
>
> It is a bug. If it is documented it is a documented bug. I was hit by it once
> and spent a day or so to find it, really nasty. I cannot agree with
> the "historical" reason. If something is wrong it it wrong and it doesn't
> matter how long it has been wrong before.

there is a very long thead in the archives about this very thing. my
stance is still what I wrote at the bottom here:

http://marc.info/?l=apache-modperl-dev&m=114987370431251&w=2

in short, we have an obligation to not bork our existing userbase who
may have been (perhaps unknowingly) taking advantage of this unique
feature of pnotes() for years and years.

--Geoff

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe [at] perl
For additional commands, e-mail: dev-help [at] perl


gozer at ectoplasm

Oct 17, 2007, 12:38 PM

Post #5 of 5 (1504 views)
Permalink
Re: [Fwd: [rt.cpan.org #30061] $r->pnotes stores aliases not copies so is subject to action at a distance] [In reply to]

Geoffrey Young wrote:
>
> Torsten Foertsch wrote:
>> On Wednesday 17 October 2007 18:10, Philippe M. Chiasson wrote:
>>> Yeah, I know. If you ask me, this pnotes behaviour is wrong, but we chose
>>> to keep it around for historical reasons. It's not a bug if it's documented
>> It is a bug. If it is documented it is a documented bug. I was hit by it once
>> and spent a day or so to find it, really nasty. I cannot agree with
>> the "historical" reason. If something is wrong it it wrong and it doesn't
>> matter how long it has been wrong before.
>
> there is a very long thead in the archives about this very thing. my
> stance is still what I wrote at the bottom here:
>
> http://marc.info/?l=apache-modperl-dev&m=114987370431251&w=2
>
> in short, we have an obligation to not bork our existing userbase who
> may have been (perhaps unknowingly) taking advantage of this unique
> feature of pnotes() for years and years.

I buy that for mod_perl 1.x, but for 2.x, not as much so.

------------------------------------------------------------------------
Philippe M. Chiasson GPG: F9BFE0C2480E7680 1AE53631CB32A107 88C3A5A5
http://gozer.ectoplasm.org/ m/gozer\@(apache|cpan|ectoplasm)\.org/
Attachments: signature.asc (0.24 KB)

ModPerl dev RSS feed   Index | Next | Previous | View Threaded
 
 


Interested in having your list archived? Contact Gossamer Threads
 
  Web Applications & Managed Hosting Powered by Gossamer Threads Inc.