Skip Menu |
Report information
Id: 60374
Status: resolved
Priority: 0/
Queue: perl5

Owner: Nobody
Requestors: badalex <badalex [at] gmail.com>
Cc:
AdminCc:

Operating System: (no value)
PatchStatus: (no value)
Severity: low
Type: unknown
Perl Version: (no value)
Fixed In: (no value)



Subject: Safe.pm sort {} bug with -Dusethreads
Date: Thu, 6 Nov 2008 08:34:44 -0700
To: perlbug [...] perl.org
From: "Alex Hunsaker" <badalex [...] gmail.com>
Download (untitled) / with headers
text/plain 798b
When using a threading perl and you reval an anon sub that contains a sort, a/$b loose their values. Both 5.8.8 and 5.10.0 seem to be affected. (see attached perl.v for perl -V output) -Dusethreads perl tsafe.pl debug: debug: debug: debug: debug: debug: debug: debug: debug: 5 4 3 2 1 without -Dusethreads perl1 tsafe.pl debug: 4 debug: 2 debug: 2 debug: 3 debug: 1 debug: 1 2 3 4 5 tsafe.pl ========== use Safe; my $safe = Safe->new('PLPerl'); $safe->permit_only(':default'); $safe->permit(qw(sort)); $safe->share(qw(&debug)); sub debug { print "debug: ". (shift) . "\n"; } my $func = $safe->reval(<<'z'); sub { my @c = sort { debug("$b"); "$a" <=> $b } qw(5 4 3 2 1); debug(join(" ", @c)); return; } z #my $f = eval 'package PLPerl; sub { @_=(); $func->(); }'; $func->(); die $@ if($@);
Download perl.v
application/octet-stream 2.9k

Message body not shown because it is not plain text.

Download perl1.v
application/octet-stream 2.1k

Message body not shown because it is not plain text.

Download tsafe.pl
text/x-perl 406b

Message body is not shown because sender requested not to inline it.

RT-Send-CC: andrew [...] tao11.riddles.org.uk
Download (untitled) / with headers
text/plain 2.2k
Andrew Gierth <andrew@tao11.riddles.org.uk> managed to track it down his explanation is below: If it helps any, I've tracked down in the perl guts exactly why this happens: cop.h: struct cop { BASEOP char * cop_label; /* label for this construct */ #ifdef USE_ITHREADS char * cop_stashpv; /* package line was compiled in */ char * cop_file; /* file name the following line # is from */ #else HV * cop_stash; /* package line was compiled in */ GV * cop_filegv; /* file the following line # is from */ #endif U32 cop_seq; /* parse sequence number */ I32 cop_arybase; /* array base this line was compiled with */ line_t cop_line; /* line # of this command */ SV * cop_warnings; /* lexical warnings bitmask */ SV * cop_io; /* lexical IO defaults */ }; A COP in perl is a control operation, basically a compiled statement, and the pointer to the current COP is used to determine all the lexical state, including the current package. pp_sort uses CopSTASH(PL_curcop) to get the package stash (symbol table) in order to locate the $a and $b variables in it. Notice, though, that without ithreads, the COP points directly to the stash, but with ithreads, it points instead to the _name_ of the stash (e.g. "main"). The problem arises because with Safe in use, the package created by Safe to use as a container _thinks_ that its name is "main" even though it's not, so the COPs compiled inside it point to the name "main" rather than to the real name of the container. So with ithreads enabled, pp_sort looks up the package stash by name, gets the "main" package rather than the safe container, and creates $main::a and $main::b to store the comparison values in. But the compiled comparison block has its own references to the variables which refers to the correct stash, so it all goes Horribly Wrong at that point. So there are three factors involved: 1) the change in layout of COP with ithreads enabled 2) the fact that Safe changes the internally-seen name of a package 3) any operation that relies on CopSTASH(PL_curcop) (I can only find a few: sort, reset, and bless) will then behave incorrectly However, I have no idea why Perl has this difference between threaded and non-threaded code.
Subject: Re: [perl #60374] perlbug AutoReply: Safe.pm sort {} bug with -Dusethreads
Date: Thu, 6 Nov 2008 09:12:25 -0700
To: perlbug-followup [...] perl.org
From: "Alex Hunsaker" <badalex [...] gmail.com>
Download (untitled) / with headers
text/plain 2.2k
Andrew Gierth <andrew@tao11.riddles.org.uk> managed to track it down his explanation is below: If it helps any, I've tracked down in the perl guts exactly why this happens: cop.h: struct cop { BASEOP char * cop_label; /* label for this construct */ #ifdef USE_ITHREADS char * cop_stashpv; /* package line was compiled in */ char * cop_file; /* file name the following line # is from */ #else HV * cop_stash; /* package line was compiled in */ GV * cop_filegv; /* file the following line # is from */ #endif U32 cop_seq; /* parse sequence number */ I32 cop_arybase; /* array base this line was compiled with */ line_t cop_line; /* line # of this command */ SV * cop_warnings; /* lexical warnings bitmask */ SV * cop_io; /* lexical IO defaults */ }; A COP in perl is a control operation, basically a compiled statement, and the pointer to the current COP is used to determine all the lexical state, including the current package. pp_sort uses CopSTASH(PL_curcop) to get the package stash (symbol table) in order to locate the $a and $b variables in it. Notice, though, that without ithreads, the COP points directly to the stash, but with ithreads, it points instead to the _name_ of the stash (e.g. "main"). The problem arises because with Safe in use, the package created by Safe to use as a container _thinks_ that its name is "main" even though it's not, so the COPs compiled inside it point to the name "main" rather than to the real name of the container. So with ithreads enabled, pp_sort looks up the package stash by name, gets the "main" package rather than the safe container, and creates $main::a and $main::b to store the comparison values in. But the compiled comparison block has its own references to the variables which refers to the correct stash, so it all goes Horribly Wrong at that point. So there are three factors involved: 1) the change in layout of COP with ithreads enabled 2) the fact that Safe changes the internally-seen name of a package 3) any operation that relies on CopSTASH(PL_curcop) (I can only find a few: sort, reset, and bless) will then behave incorrectly However, I have no idea why Perl has this difference between threaded and non-threaded code.
RT-Send-CC: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 214b
Any news of progress with this bug? Meanwhile, I have a workaround: explicitly share the globs for a and b. In other words, add *a and * b to the arguments of the share() method: $safe->share(qw(&debug *a *b));
Subject: Re: [perl #60374] Safe.pm sort {} bug with -Dusethreads
Date: Tue, 6 Oct 2009 14:33:14 -0600
To: perlbug-followup [...] perl.org
From: Alex Hunsaker <badalex [...] gmail.com>
Download (untitled) / with headers
text/plain 1.3k
On Tue, Oct 6, 2009 at 13:55, Tim Bunce via RT <perlbug-followup@perl.org> wrote: Show quoted text
> Any news of progress with this bug?
Nope still broke for me on 5.10.1 Here are the workaround I know about: 1) dont use $a and $b in sort, use @_ 2) your way of adding *a and *b, but that only works as long as the package stays the same... 3) declare the package at the top of reval, i.e. my $safe = Safe->new('PLPerl'); $safe->permit(qw(sort)); my $func = $safe->reval(<<'eval'); package PLPerl; sub { sort { "$a" cmp "$b" } qw(works now); } eval $func->(); 4) fix perl :) as mentioned in the previous message Show quoted text
> Meanwhile, I have a workaround: explicitly share the globs for a and b. In other words, add *a > and * b to the arguments of the share() method: > > $safe->share(qw(&debug *a *b));
Yeah as noted above that should work. Unless you ever sort inside a different package than you gave to Safe->new() (see the below test case) use Safe(); my $safe = Safe->new('PLPerl'); $safe->permit_only(':default'); $safe->permit(qw(sort)); $safe->share(qw(&PLPerl::debug *a *b)); my $func = $safe->reval(<<'z'); sub { package PLPrl; my @c = sort { PLPerl::debug("$a ". $b); "$a" <=> $b } qw(5 4 3 2 1); PLPerl::debug(join(" ", @c)); return; } z $func->(); package PLPerl; sub debug { print "debug: ". (shift) . "\n"; } $ perl tsafe.pl debug: debug: debug: debug: debug: debug: debug: debug: debug: 5 4 3 2 1
CC: perl5-porters [...] perl.org
Subject: Re: [perl #60374] Safe.pm sort {} bug with -Dusethreads
Date: Wed, 7 Oct 2009 17:22:25 +0200
To: perlbug-followup [...] perl.org
From: Rafael Garcia-Suarez <rgs [...] consttype.org>
Download (untitled) / with headers
text/plain 624b
2009/10/6 Tim Bunce via RT <perlbug-followup@perl.org>: Show quoted text
> Any news of progress with this bug? > > Meanwhile, I have a workaround: explicitly share the globs for a and b. In other words, add *a > and * b to the arguments of the share() method: > > $safe->share(qw(&debug *a *b));
I don't think that *a and *b should be added to the default sharelist of Safe. Actually the only glob there is *_, and I think it should remain as is. The bug is most likely in the sort implementation, which is notoriously thread-unsafe when it invokes sort subs. In short, my advice would be : don't use sort() at all with a threaded perl.
CC: perlbug-followup [...] perl.org, perl5-porters [...] perl.org
Subject: Re: [perl #60374] Safe.pm sort {} bug with -Dusethreads
Date: Wed, 7 Oct 2009 09:13:58 -0700
To: Rafael Garcia-Suarez <rgs [...] consttype.org>
From: "David E. Wheeler" <david [...] kineticode.com>
Download (untitled) / with headers
text/plain 391b
On Oct 7, 2009, at 8:22 AM, Rafael Garcia-Suarez wrote: Show quoted text
> The bug is most likely in the sort implementation, which is > notoriously thread-unsafe when it invokes sort subs. > > In short, my advice would be : don't use sort() at all with a > threaded perl.
Say what? Sure, on current releases, but surely that should be fixed. There are more and more threaded Perls every day… David
Download (untitled) / with headers
text/plain 380b
On Wed Oct 07 08:22:57 2009, rgs@consttype.org wrote: Show quoted text
> The bug is most likely in the sort implementation, which is > notoriously thread-unsafe when it invokes sort subs. > > In short, my advice would be : don't use sort() at all with a threaded > perl.
Um... you do realize its just a threaded *build* of perl. Nothing here is actually threaded or even using threads... Right?
CC: perlbug-followup [...] perl.org, perl5-porters [...] perl.org
Subject: Re: [perl #60374] Safe.pm sort {} bug with -Dusethreads
Date: Wed, 7 Oct 2009 19:24:02 +0200
To: "David E. Wheeler" <david [...] kineticode.com>
From: Rafael Garcia-Suarez <rgs [...] consttype.org>
Download (untitled) / with headers
text/plain 778b
2009/10/7 David E. Wheeler <david@kineticode.com>: Show quoted text
> On Oct 7, 2009, at 8:22 AM, Rafael Garcia-Suarez wrote: >
>> The bug is most likely in the sort implementation, which is >> notoriously thread-unsafe when it invokes sort subs. >> >> In short, my advice would be : don't use sort() at all with a threaded >> perl.
> > Say what? Sure, on current releases, but surely that should be fixed. There > are more and more threaded Perls every day…
Even if we manage someone to fix threading, this will slow down sort(). Threading also has a non-negligeable performance impact on memory and speed of every perl program. I think we should advice people in INSTALL to build with threading only if they really need it, and tell OS vendors to disable threading on their system perls.
CC: perlbug-followup [...] perl.org, perl5-porters [...] perl.org
Subject: Re: [perl #60374] Safe.pm sort {} bug with -Dusethreads
Date: Wed, 7 Oct 2009 10:26:44 -0700
To: Rafael Garcia-Suarez <rgs [...] consttype.org>
From: "David E. Wheeler" <david [...] kineticode.com>
Download (untitled) / with headers
text/plain 523b
On Oct 7, 2009, at 10:24 AM, Rafael Garcia-Suarez wrote: Show quoted text
> Even if we manage someone to fix threading, this will slow down > sort(). Threading also has a non-negligeable performance impact on > memory and speed of every perl program. > > I think we should advice people in INSTALL to build with threading > only if they really need it, and tell OS vendors to disable threading > on their system perls.
Snow Leopard ships with a threaded Perl. Hell, I think Leopard did. That cat might have left the bag. Best, David
CC: <perl5-porters [...] perl.org>
Subject: RE: [perl #60374] Safe.pm sort {} bug with -Dusethreads
Date: Wed, 7 Oct 2009 18:17:15 -0700
To: "'Rafael Garcia-Suarez'" <rgs [...] consttype.org>, <perlbug-followup [...] perl.org>
From: "Jan Dubois" <jand [...] activestate.com>
Download (untitled) / with headers
text/plain 462b
On Wed, 07 Oct 2009, Rafael Garcia-Suarez wrote: Show quoted text
> > The bug is most likely in the sort implementation, which is > notoriously thread-unsafe when it invokes sort subs. > > In short, my advice would be : don't use sort() at all with a threaded perl.
Just to be sure: That advice is in the context of using Safe.pm, right? Or are there problems with sort() ourside safe compartments under threading as well? Are there any open bugs about them? Cheers, -Jan
CC: perlbug-followup [...] perl.org, perl5-porters [...] perl.org
Subject: Re: [perl #60374] Safe.pm sort {} bug with -Dusethreads
Date: Thu, 8 Oct 2009 08:25:58 +0200
To: Jan Dubois <jand [...] activestate.com>
From: Rafael Garcia-Suarez <rgs [...] consttype.org>
Download (untitled) / with headers
text/plain 714b
2009/10/8 Jan Dubois <jand@activestate.com>: Show quoted text
> On Wed, 07 Oct 2009, Rafael Garcia-Suarez wrote:
>> >> The bug is most likely in the sort implementation, which is >> notoriously thread-unsafe when it invokes sort subs. >> >> In short, my advice would be : don't use sort() at all with a threaded perl.
> > Just to be sure: That advice is in the context of using Safe.pm, > right?  Or are there problems with sort() ourside safe compartments > under threading as well?  Are there any open bugs about them?
No, sort is thread-unsafe as long as the sort function is custom (and not XS, IIRC). That's basically because it's called in a super-fast way, for speed. I think there are bugs, but can't find one rapidly.
CC: Jan Dubois <jand [...] activestate.com>, perlbug-followup [...] perl.org, perl5-porters [...] perl.org
Subject: Re: [perl #60374] Safe.pm sort {} bug with -Dusethreads
Date: Thu, 8 Oct 2009 12:56:59 +0200
To: Rafael Garcia-Suarez <rgs [...] consttype.org>
From: demerphq <demerphq [...] gmail.com>
Download (untitled) / with headers
text/plain 1002b
2009/10/8 Rafael Garcia-Suarez <rgs@consttype.org>: Show quoted text
> 2009/10/8 Jan Dubois <jand@activestate.com>:
>> On Wed, 07 Oct 2009, Rafael Garcia-Suarez wrote:
>>> >>> The bug is most likely in the sort implementation, which is >>> notoriously thread-unsafe when it invokes sort subs. >>> >>> In short, my advice would be : don't use sort() at all with a threaded perl.
>> >> Just to be sure: That advice is in the context of using Safe.pm, >> right?  Or are there problems with sort() ourside safe compartments >> under threading as well?  Are there any open bugs about them?
> > No, sort is thread-unsafe as long as the sort function is custom (and > not XS, IIRC). That's basically because it's called in a super-fast > way, for speed. I think there are bugs, but can't find one rapidly.
Considering all Win32 builds pretty much are threaded id be surprised if it was that easy. Having said that, maybe this explains some weird win32 bugs... Yves -- perl -Mre=debug -e "/just|another|perl|hacker/"
Subject: Re: [perl #60374] Safe.pm sort {} bug with -Dusethreads
Date: Thu, 08 Oct 2009 09:58:28 -0600
To: "Perl5-Porters" <perl5-porters [...] perl.org>
From: "Curtis Jewell" <lists.perl.perl5-porters [...] csjewell.fastmail.us>
On Wed, 07 Oct 2009 10:26 -0700, "David E. Wheeler" <david@kineticode.com> wrote: Show quoted text
> On Oct 7, 2009, at 10:24 AM, Rafael Garcia-Suarez wrote: >
> > Even if we manage someone to fix threading, this will slow down > > sort(). Threading also has a non-negligeable performance impact on > > memory and speed of every perl program. > > > > I think we should advice people in INSTALL to build with threading > > only if they really need it, and tell OS vendors to disable threading > > on their system perls.
> > Snow Leopard ships with a threaded Perl. Hell, I think Leopard did. > That cat might have left the bag.
And Strawberry is threaded. If I switched it to non-threaded now, I'd be stood up to a wall and shot by the Padre people (they require threading) among others. --Curtis -- Curtis Jewell csjewell@cpan.org http://csjewell.dreamwidth.org/ perl@csjewell.fastmail.us http://csjewell.comyr.org/perl/ "Your random numbers are not that random" -- perl-5.10.1.tar.gz/util.c Strawberry Perl for Windows betas: http://strawberryperl.com/beta/
CC: Jan Dubois <jand [...] activestate.com>, perlbug-followup [...] perl.org, perl5-porters [...] perl.org
Subject: Re: [perl #60374] Safe.pm sort {} bug with -Dusethreads
Date: Thu, 8 Oct 2009 09:41:42 -0700
To: Rafael Garcia-Suarez <rgs [...] consttype.org>
From: "David E. Wheeler" <david [...] kineticode.com>
Download (untitled) / with headers
text/plain 647b
On Oct 7, 2009, at 11:25 PM, Rafael Garcia-Suarez wrote: Show quoted text
>> Just to be sure: That advice is in the context of using Safe.pm, >> right? Or are there problems with sort() ourside safe compartments >> under threading as well? Are there any open bugs about them?
> > No, sort is thread-unsafe as long as the sort function is custom (and > not XS, IIRC). That's basically because it's called in a super-fast > way, for speed. I think there are bugs, but can't find one rapidly.
It seems to me that such bugs should be fixed as they're identified. Even if sorting becomes slower with threads, I'll take correct over fast any day. Best, David
CC: perl5-porters [...] perl.org
Subject: Re: [perl #60374] Safe.pm sort {} bug with -Dusethreads
Date: Thu, 8 Oct 2009 19:00:34 +0200
To: "David E. Wheeler" <david [...] kineticode.com>
From: Rafael Garcia-Suarez <rgs [...] consttype.org>
Download (untitled) / with headers
text/plain 807b
2009/10/8 David E. Wheeler <david@kineticode.com>: Show quoted text
> On Oct 7, 2009, at 11:25 PM, Rafael Garcia-Suarez wrote: >
>>> Just to be sure: That advice is in the context of using Safe.pm, >>> right?  Or are there problems with sort() ourside safe compartments >>> under threading as well?  Are there any open bugs about them?
>> >> No, sort is thread-unsafe as long as the sort function is custom (and >> not XS, IIRC). That's basically because it's called in a super-fast >> way, for speed. I think there are bugs, but can't find one rapidly.
> > It seems to me that such bugs should be fixed as they're identified. Even if > sorting becomes slower with threads, I'll take correct over fast any day.
I wholeheartedly agree. Someone should fix that. It's just not my itch: I don't build or use threaded perls.
CC: Rafael Garcia-Suarez <rgs [...] consttype.org>, Jan Dubois <jand [...] activestate.com>, perlbug-followup [...] perl.org, perl5-porters [...] perl.org
Subject: Re: [perl #60374] Safe.pm sort {} bug with -Dusethreads
Date: Thu, 8 Oct 2009 19:05:01 +0200
To: "David E. Wheeler" <david [...] kineticode.com>
From: "H.Merijn Brand" <h.m.brand [...] xs4all.nl>
Download (untitled) / with headers
text/plain 1.4k
On Thu, 8 Oct 2009 09:41:42 -0700, "David E. Wheeler" <david@kineticode.com> wrote: Show quoted text
> On Oct 7, 2009, at 11:25 PM, Rafael Garcia-Suarez wrote: >
> >> Just to be sure: That advice is in the context of using Safe.pm, > >> right? Or are there problems with sort() ourside safe compartments > >> under threading as well? Are there any open bugs about them?
> > > > No, sort is thread-unsafe as long as the sort function is custom (and > > not XS, IIRC). That's basically because it's called in a super-fast > > way, for speed. I think there are bugs, but can't find one rapidly.
> > It seems to me that such bugs should be fixed as they're identified. > Even if sorting becomes slower with threads, I'll take correct over > fast any day.
Then use an unthreaded perl, and you've got correct behaviour :) As long as it doesn't slow down unthreaded builds, I'm fine with any solution. I have never ever needed a threaded build for anything, and speed is the major reason to build unthreaded perl from the beginning. IIRC the worst case difference was up to 40% loss (gcc + threads vs native cc no-threads), where threads was the biggest problem. -- H.Merijn Brand http://tux.nl Perl Monger http://amsterdam.pm.org/ using & porting perl 5.6.2, 5.8.x, 5.10.x, 5.11.x on HP-UX 10.20, 11.00, 11.11, 11.23, and 11.31, OpenSuSE 10.3, 11.0, and 11.1, AIX 5.2 and 5.3. http://mirrors.develooper.com/hpux/ http://www.test-smoke.org/ http://qa.perl.org http://www.goldmark.org/jeff/stupid-disclaimers/
CC: Perl5-Porters <perl5-porters [...] perl.org>
Subject: Re: [perl #60374] Safe.pm sort {} bug with -Dusethreads
Date: Thu, 8 Oct 2009 20:59:47 +0200
To: Curtis Jewell <lists.perl.perl5-porters [...] csjewell.fastmail.us>
From: demerphq <demerphq [...] gmail.com>
Download (untitled) / with headers
text/plain 1009b
2009/10/8 Curtis Jewell <lists.perl.perl5-porters@csjewell.fastmail.us>: Show quoted text
> On Wed, 07 Oct 2009 10:26 -0700, "David E. Wheeler" > <david@kineticode.com> wrote:
>> On Oct 7, 2009, at 10:24 AM, Rafael Garcia-Suarez wrote: >>
>> > Even if we manage someone to fix threading, this will slow down >> > sort(). Threading also has a non-negligeable performance impact on >> > memory and speed of every perl program. >> > >> > I think we should advice people in INSTALL to build with threading >> > only if they really need it, and tell OS vendors to disable threading >> > on their system perls.
>> >> Snow Leopard ships with a threaded Perl. Hell, I think Leopard did. >> That cat might have left the bag.
> > And Strawberry is threaded.  If I switched it to non-threaded now, I'd > be stood up to a wall and shot by the Padre people (they require > threading) among others.
Im inclined to say that a non-threaded win32 build is pretty close to broken. Yves -- perl -Mre=debug -e "/just|another|perl|hacker/"
Download (untitled) / with headers
text/plain 1.5k
On Wed Oct 07 08:22:57 2009, rgs@consttype.org wrote: Show quoted text
> I don't think that *a and *b should be added to the default sharelist > of Safe. Actually the only glob there is *_, and I think it should > remain as is.
Hrm... Ok How does the below look to you? Safe.pm | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) Fix ->reval('sub { sort { "$a" <=> "$b" } }')->() on usethreads This is broken on usethreads builds because we only store the text of the stash/package (i.e. main) in COP's. When the anon sub executes, it then uses the stash name to look stuff up using the current (and wrong) PL_defstash/PL_curstash. The non threaded case stores references so its not an issue. To fix this modify ->reval() to detect if we are returning an anon sub. If so return an anon sub which calls _safe_call_sv(..., $real_anon_sub) to setup the right PL_defstash and friends. -- --- /usr/lib/perl5/site_perl/5.10.1/Safe.pm 2009-08-25 01:41:11.000000000 -0600 +++ 5.10.1/Safe.pm 2009-10-08 23:07:33.027744998 -0600 @@ -2,6 +2,7 @@ use 5.003_11; use strict; +use Config qw(%Config); $Safe::VERSION = "2.19"; @@ -289,7 +290,13 @@ my $root = $obj->{Root}; my $evalsub = lexless_anon_sub($root,$strict, $expr); - return Opcode::_safe_call_sv($root, $obj->{Mask}, $evalsub); + my $ret = Opcode::_safe_call_sv($root, $obj->{Mask}, $evalsub); + + if ($Config{usethreads} && ref $ret eq 'CODE') { + return sub { Opcode::_safe_call_sv($root, $obj->{Mask}, $ret) }; + } + + return $ret; } sub rdo {
CC: "Perl5-Porters" <perl5-porters [...] perl.org>
Subject: RE: [perl #60374] Safe.pm sort {} bug with -Dusethreads
Date: Fri, 9 Oct 2009 08:24:34 +0100
To: "demerphq" <demerphq [...] gmail.com>, "Curtis Jewell" <lists.perl.perl5-porters [...] csjewell.fastmail.us>
From: "Steve Hay" <SteveHay [...] planit.com>
Download (untitled) / with headers
text/plain 1.3k
demerphq wrote on 2009-10-08: Show quoted text
> 2009/10/8 Curtis Jewell <lists.perl.perl5-porters@csjewell.fastmail.us>:
>> On Wed, 07 Oct 2009 10:26 -0700, "David E. Wheeler" >> <david@kineticode.com> wrote:
>>> On Oct 7, 2009, at 10:24 AM, Rafael Garcia-Suarez wrote: >>>
>>>> Even if we manage someone to fix threading, this will slow down >>>> sort(). Threading also has a non-negligeable performance impact on >>>> memory and speed of every perl program. >>>> >>>> I think we should advice people in INSTALL to build with threading >>>> only if they really need it, and tell OS vendors to disable threading >>>> on their system perls.
>>> >>> Snow Leopard ships with a threaded Perl. Hell, I think Leopard did. >>> That cat might have left the bag.
>> And Strawberry is threaded.  If I switched it to non-threaded now, I'd >> be stood up to a wall and shot by the Padre people (they require >> threading) among others.
> Im inclined to say that a non-threaded win32 build is pretty close to > broken. >
Why do you say that? I always used to use a non-threaded perl on Win32, and only switched to threaded when I had to because mod_perl-2 needs it. Even now I still don't enable PERL_IMPLICIT_SYS because I have no need for the fork() emulation, and prefer Perl's malloc() (which can't be enabled with PERL_IMPLICIT_SYS) to the CRT's version.
CC: Curtis Jewell <lists.perl.perl5-porters [...] csjewell.fastmail.us>, Perl5-Porters <perl5-porters [...] perl.org>
Subject: Re: [perl #60374] Safe.pm sort {} bug with -Dusethreads
Date: Fri, 9 Oct 2009 09:44:17 +0200
To: Steve Hay <SteveHay [...] planit.com>
From: demerphq <demerphq [...] gmail.com>
Download (untitled) / with headers
text/plain 1.5k
2009/10/9 Steve Hay <SteveHay@planit.com>: Show quoted text
> demerphq wrote on 2009-10-08:
>> 2009/10/8 Curtis Jewell <lists.perl.perl5-porters@csjewell.fastmail.us>:
>>> On Wed, 07 Oct 2009 10:26 -0700, "David E. Wheeler" >>> <david@kineticode.com> wrote:
>>>> On Oct 7, 2009, at 10:24 AM, Rafael Garcia-Suarez wrote: >>>>
>>>>> Even if we manage someone to fix threading, this will slow down >>>>> sort(). Threading also has a non-negligeable performance impact on >>>>> memory and speed of every perl program. >>>>> >>>>> I think we should advice people in INSTALL to build with threading >>>>> only if they really need it, and tell OS vendors to disable threading >>>>> on their system perls.
>>>> >>>> Snow Leopard ships with a threaded Perl. Hell, I think Leopard did. >>>> That cat might have left the bag.
>>>  And Strawberry is threaded.  If I switched it to non-threaded now, I'd >>> be stood up to a wall and shot by the Padre people (they require >>> threading) among others.
>>  Im inclined to say that a non-threaded win32 build is pretty close to >> broken. >>
> > Why do you say that? I always used to use a non-threaded perl on Win32, and only switched to threaded when I had to because mod_perl-2 needs it. Even now I still don't enable PERL_IMPLICIT_SYS because I have no need for the fork() emulation, and prefer Perl's malloc() (which can't be enabled with PERL_IMPLICIT_SYS) to the CRT's version.
Ah well, then its just me. Anytime i used an unthreaded perl on windows it was just pain. Yves -- perl -Mre=debug -e "/just|another|perl|hacker/"
Subject: Re: [perl #60374] Safe.pm sort {} bug with -Dusethreads
Date: Fri, 9 Oct 2009 06:22:49 -0400 (EDT)
To: perl5-porters [...] perl.org
From: "John P. Linderman" <jpl [...] research.att.com>
Download (untitled) / with headers
text/plain 992b
David E. Wheeler noted: Show quoted text
> On Oct 7, 2009, at 11:25 PM, Rafael Garcia-Suarez wrote: >
> >> Just to be sure: That advice is in the context of using Safe.pm, > >> right? Or are there problems with sort() ourside safe compartments > >> under threading as well? Are there any open bugs about them?
> > > > No, sort is thread-unsafe as long as the sort function is custom (and > > not XS, IIRC). That's basically because it's called in a super-fast > > way, for speed. I think there are bugs, but can't find one rapidly.
> > It seems to me that such bugs should be fixed as they're identified. > Even if sorting becomes slower with threads, I'll take correct over > fast any day.
I have never used threads, and I don't understand the details of the problem caused by custom comparison routines, but I have never let ignorance stand in the way of an opinion. Might it be possible to put a global shared lock on the use of sort, so only one thread at a time could be doing a sort? -- jpl
CC: perl5-porters [...] perl.org
Subject: Re: [perl #60374] Safe.pm sort {} bug with -Dusethreads
Date: Fri, 9 Oct 2009 12:37:15 +0200
To: "John P. Linderman" <jpl [...] research.att.com>
From: Rafael Garcia-Suarez <rgs [...] consttype.org>
Download (untitled) / with headers
text/plain 1.2k
2009/10/9 John P. Linderman <jpl@research.att.com>: Show quoted text
> David E. Wheeler noted:
>> On Oct 7, 2009, at 11:25 PM, Rafael Garcia-Suarez wrote: >>
>> >> Just to be sure: That advice is in the context of using Safe.pm, >> >> right?  Or are there problems with sort() ourside safe compartments >> >> under threading as well?  Are there any open bugs about them?
>> > >> > No, sort is thread-unsafe as long as the sort function is custom (and >> > not XS, IIRC). That's basically because it's called in a super-fast >> > way, for speed. I think there are bugs, but can't find one rapidly.
>> >> It seems to me that such bugs should be fixed as they're identified. >> Even if sorting becomes slower with threads, I'll take correct over >> fast any day.
> > I have never used threads, and I don't understand the details of > the problem caused by custom comparison routines, but I have > never let ignorance stand in the way of an opinion.  Might it be > possible to put a global shared lock on the use of sort, so only > one thread at a time could be doing a sort?  -- jpl
That way you will deadlock if your sort subroutine wants to wait for another thread. (OK, that's probably very unlikely). IIRC, the problem comes from the fact that pp_sort modifies the optree, which is shared among threads.
CC: "John P. Linderman" <jpl [...] research.att.com>, perl5-porters [...] perl.org
Subject: Re: [perl #60374] Safe.pm sort {} bug with -Dusethreads
Date: Sun, 11 Oct 2009 13:35:40 +0100
To: Rafael Garcia-Suarez <rgs [...] consttype.org>
From: Dave Mitchell <davem [...] iabyn.com>
Download (untitled) / with headers
text/plain 466b
On Fri, Oct 09, 2009 at 12:37:15PM +0200, Rafael Garcia-Suarez wrote: Show quoted text
> That way you will deadlock if your sort subroutine wants to wait for > another thread. (OK, that's probably very unlikely). > IIRC, the problem comes from the fact that pp_sort modifies the > optree, which is shared among threads.
Are you sure that's still the case? I thought that had been fixed ages ago? -- Dave's first rule of Opera: If something needs saying, say it: don't warble it.
CC: Rafael Garcia-Suarez <rgs [...] consttype.org>, "John P. Linderman" <jpl [...] research.att.com>, perl5-porters [...] perl.org
Subject: Re: [perl #60374] Safe.pm sort {} bug with -Dusethreads
Date: Fri, 30 Oct 2009 15:55:00 +0000
To: Dave Mitchell <davem [...] iabyn.com>
From: Tim Bunce <Tim.Bunce [...] pobox.com>
Download (untitled) / with headers
text/plain 550b
On Sun, Oct 11, 2009 at 01:35:40PM +0100, Dave Mitchell wrote: Show quoted text
> On Fri, Oct 09, 2009 at 12:37:15PM +0200, Rafael Garcia-Suarez wrote:
> > That way you will deadlock if your sort subroutine wants to wait for > > another thread. (OK, that's probably very unlikely). > > IIRC, the problem comes from the fact that pp_sort modifies the > > optree, which is shared among threads.
> > Are you sure that's still the case? I thought that had been fixed ages > ago?
Specifically in http://rt.perl.org/rt3/Public/Bug/Display.html?id=30333 I presume. Tim.
CC: perl5-porters [...] perl.org
Subject: Re: [perl #60374] Safe.pm sort {} bug with -Dusethreads
Date: Fri, 6 Nov 2009 10:43:40 +0000
To: Tim Bunce via RT <perlbug-followup [...] perl.org>
From: Tim Bunce <Tim.Bunce [...] pobox.com>
On Tue, Oct 06, 2009 at 12:55:47PM -0700, Tim Bunce via RT wrote: Show quoted text
> Any news of progress with this bug? > > Meanwhile, I have a workaround: explicitly share the globs for a and b. In other words, add *a > and * b to the arguments of the share() method: > > $safe->share(qw(&debug *a *b));
To summarize the thread so far... - it's confirmed that the bug is still present in 5.10.1. - we digressed into a "don't use threaded perl" discussion but that's not an option for many people these days. - some workarounds are possible, but *none work* for calls to sort() with custom sort subs in third-party code in other packages e.g., CPAN modules. Tim. p.s. It was suggested that sort with custom sort subs isn't thread safe anyway. If that's true then it's a very serious problem. It's covered by RT#30333 (http://rt.perl.org/rt3/Public/Bug/Display.html?id=30333) which is still marked Open. Yet I see tests for sort with a custom sub in op/threads.t. Should RT#30333 be marked Closed? If so, does that also fix RT#37508? http://rt.perl.org/rt3/Public/Bug/Display.html?id=37508
CC: Tim Bunce via RT <perlbug-followup [...] perl.org>, perl5-porters [...] perl.org
Subject: Re: [perl #60374] Safe.pm sort {} bug with -Dusethreads
Date: Fri, 6 Nov 2009 14:03:43 -0500
To: Tim Bunce <Tim.Bunce [...] pobox.com>
From: Eric Brine <ikegami [...] adaelis.com>
Download (untitled) / with headers
text/plain 859b
On Fri, Nov 6, 2009 at 5:43 AM, Tim Bunce <Tim.Bunce@pobox.com> wrote: Show quoted text
> p.s. It was suggested that sort with custom sort subs isn't thread safe > anyway. If that's true then it's a very serious problem. It's covered > by RT#30333 (http://rt.perl.org/rt3/Public/Bug/Display.html?id=30333) > which is still marked Open. Yet I see tests for sort with a custom sub > in op/threads.t. Should RT#30333 be marked Closed? >
I can't reproduce it with new versions of Perl. The caused identified in RT#30333 was solved by http://perl5.git.perl.org/perl.git/commit/9850bf21fc4ed69d8ddb0293df59411f891c62df If so, does that also fix RT#37508? Show quoted text
I can't reproduce it with new versions of Perl. The caused identified in RT#37508 was the same one identified in RT#30333. I think both should be closed. - Eric
CC: Tim Bunce via RT <perlbug-followup [...] perl.org>, perl5-porters [...] perl.org
Subject: Re: [perl #60374] Safe.pm sort {} bug with -Dusethreads
Date: Sat, 7 Nov 2009 10:57:22 -0700
To: Tim Bunce <Tim.Bunce [...] pobox.com>
From: Alex Hunsaker <badalex [...] gmail.com>
Download (untitled) / with headers
text/plain 1.2k
[ Resend I replied but RT does not show it yet +20 hours and neither does any perl5-porters list i can see ] On Fri, Nov 6, 2009 at 03:43, Tim Bunce <Tim.Bunce@pobox.com> wrote: Show quoted text
> On Tue, Oct 06, 2009 at 12:55:47PM -0700, Tim Bunce via RT wrote:
Show quoted text
> To summarize the thread so far... > - it's confirmed that the bug is still present in 5.10.1. > - we digressed into a "don't use threaded perl" discussion >    but that's not an option for many people these days.
Yes, and thanks for pointing out that the above really has nothing to do with *this* bug. Its not running a sort with threads its just broken with a threaded *build* of perl. :) Show quoted text
> - some workarounds are possible, but *none work* for calls to > sort() with custom sort subs in third-party code
Um the last patch I posted should... http://rt.perl.org/rt3/Ticket/Display.html?id=60374#txn-612980. And while I found it quite ugly and im sure there are better ways to fix this... (namely we could fix Opcode::_safe_call_sv to do something similar, or fix it as diagnosed in http://rt.perl.org/rt3/Ticket/Display.html?id=60374#txn-491794) I was more or less looking for some kind of validation as to the approach :). But it seems RT never sent it anywhere... Mea culpa
CC: Tim Bunce via RT <perlbug-followup [...] perl.org>, perl5-porters [...] perl.org
Subject: Re: [perl #60374] Safe.pm sort {} bug with -Dusethreads
Date: Sat, 7 Nov 2009 11:17:54 -0700
To: Tim Bunce <Tim.Bunce [...] pobox.com>
From: Alex Hunsaker <badalex [...] gmail.com>
Download (untitled) / with headers
text/plain 1.5k
On Sat, Nov 7, 2009 at 10:57, Alex Hunsaker <badalex@gmail.com> wrote: Show quoted text
> Um the last patch I posted should... > http://rt.perl.org/rt3/Ticket/Display.html?id=60374#txn-612980.  And > while I found it quite ugly
For reference here is said patch also find attached for the inevitable gmail white space damage. Subject: Fix ->reval('sub { sort { "$a" <=> "$b" } }')->() on usethreads This is broken on usethreads builds because we only store the text of the stash/package (i.e. main) in COP's. When the anon sub executes, it then uses the stash name to look stuff up using the current (and wrong) PL_defstash/PL_curstash. The non threaded case stores references so its not an issue. To fix this modify ->reval() to detect if we are returning an anon sub. If so return an anon sub which calls _safe_call_sv(..., $real_anon_sub) to setup the right PL_defstash and friends. -- Safe.pm | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/ext/Safe/Safe.pm b/ext/Safe/Safe.pm index ff099ec..e9c5dfa 100644 --- a/ext/Safe/Safe.pm +++ b/ext/Safe/Safe.pm @@ -2,6 +2,7 @@ package Safe; use 5.003_11; use strict; +use Config qw(%Config); $Safe::VERSION = "2.18"; @@ -289,7 +290,13 @@ sub reval { my $root = $obj->{Root}; my $evalsub = lexless_anon_sub($root,$strict, $expr); - return Opcode::_safe_call_sv($root, $obj->{Mask}, $evalsub); + my $ret = Opcode::_safe_call_sv($root, $obj->{Mask}, $evalsub); + + if ($Config{usethreads} && ref $ret eq 'CODE') { + return sub { Opcode::_safe_call_sv($root, $obj->{Mask}, $ret) }; + } + + return $ret; } sub rdo {

Message body is not shown because sender requested not to inline it.

Subject: Re: [perl #60374] Safe.pm sort {} bug with -Dusethreads
Date: Sat, 7 Nov 2009 18:44:40 +0000
To: badalex [...] gmail.com, perl5-porters [...] perl.org
From: Ben Morrow <ben [...] morrow.me.uk>
Download (untitled) / with headers
text/plain 379b
Quoth badalex@gmail.com (Alex Hunsaker): Show quoted text
> my $evalsub = lexless_anon_sub($root,$strict, $expr); > - return Opcode::_safe_call_sv($root, $obj->{Mask}, $evalsub); > + my $ret = Opcode::_safe_call_sv($root, $obj->{Mask}, $evalsub); > + > + if ($Config{usethreads} && ref $ret eq 'CODE') {
This should use Scalar::Util::reftype, in case the coderef is blessed. Ben
Subject: Re: [perl #60374] Safe.pm sort {} bug with -Dusethreads
Date: Fri, 6 Nov 2009 14:30:47 -0700
To: perlbug-followup [...] perl.org
From: Alex Hunsaker <badalex [...] gmail.com>
Download (untitled) / with headers
text/plain 1.2k
On Fri, Nov 6, 2009 at 03:44, Tim Bunce via RT <perlbug-followup@perl.org> wrote: Show quoted text
> On Tue, Oct 06, 2009 at 12:55:47PM -0700, Tim Bunce via RT wrote:
>> Any news of progress with this bug? >>
> To summarize the thread so far... > - it's confirmed that the bug is still present in 5.10.1. > - we digressed into a "don't use threaded perl" discussion >    but that's not an option for many people these days.
Yes, and thanks for pointing out that the above really has nothing to do with *this* bug. Its not running a sort with threads its just broken with a threaded *build* of perl. :) Show quoted text
> - some workarounds are possible, but *none work* for calls to >    sort() with custom sort subs in third-party code
Um the last patch I posted should... http://rt.perl.org/rt3/Ticket/Display.html?id=60374#txn-612980. And while I found it quite ugly and im sure there are better ways to fix this... (namely we could fix Opcode::_safe_call_sv to do something similar, or fix it as diagnosed in http://rt.perl.org/rt3/Ticket/Display.html?id=60374#txn-491794) I was more or less looking for some kind of validation as to the approach :). People seemed to prefer to talk about how you should never be using a threaded perl anyways... *shrug* Perhaps I should submit a patch to disable -DUSETHREADS instead ?
CC: perlbug-followup [...] perl.org
Subject: Re: [perl #60374] Safe.pm sort {} bug with -Dusethreads
Date: Sun, 8 Nov 2009 23:36:46 +0000
To: Alex Hunsaker <badalex [...] gmail.com>
From: Tim Bunce <Tim.Bunce [...] pobox.com>
Download (untitled) / with headers
text/plain 1.6k
On Fri, Nov 06, 2009 at 02:30:47PM -0700, Alex Hunsaker wrote: Show quoted text
> > Um the last patch I posted should... > http://rt.perl.org/rt3/Ticket/Display.html?id=60374#txn-612980. And > while I found it quite ugly and im sure there are better ways to fix > this... (namely we could fix Opcode::_safe_call_sv to do something > similar, or fix it as diagnosed in > http://rt.perl.org/rt3/Ticket/Display.html?id=60374#txn-491794) > I was more or less looking for some kind of validation as to the > approach :).
I don't think the "detect if we are returning an anon sub" approach will work because the reval() may have compiled some named subs that will be invoked later. That's the case with PostgrSQL's embedded plperl, for example. http://rt.perl.org/rt3/Public/Bug/Display.html?id=60374#txn-491794 says "pp_sort uses CopSTASH(PL_curcop) to get the package stash (symbol table) in order to locate the $a and $b variables in it." While pp_sort does call CopSTASH(PL_curcop) directly, all it does with it is store the value in PL_sortstash which, as far as I can tell, isn't used or documented anywhere at all. (It could possibly be deleted.) The code to lookup the $a and $b uses: PL_firstgv = gv_fetchpvs("a", GV_ADD|GV_NOTQUAL, SVt_PV); PL_secondgv = gv_fetchpvs("b", GV_ADD|GV_NOTQUAL, SVt_PV); and it's Perl_gv_fetchpvn_flags that ends up calling CopSTASH(PL_curcop). If there was a way to get the stash pointer (not name) that was in effect at the time the code was compiled we could use that to lookup $a and $b. There are stash pointers, er, stashed in lots of places. One approach might be CvSTASH(find_runcv(...)) Tim.
CC: perlbug-followup [...] perl.org
Subject: Re: [perl #60374] Safe.pm sort {} bug with -Dusethreads
Date: Sun, 8 Nov 2009 17:56:21 -0700
To: Tim Bunce <Tim.Bunce [...] pobox.com>
From: Alex Hunsaker <badalex [...] gmail.com>
Download (untitled) / with headers
text/plain 1.3k
On Sun, Nov 8, 2009 at 16:36, Tim Bunce <Tim.Bunce@pobox.com> wrote: Show quoted text
> I don't think the "detect if we are returning an anon sub" approach will > work because the reval() may have compiled some named subs that will be > invoked later. That's the case with PostgrSQL's embedded plperl, for example.
BTW I just tested and it does fix PostgreSQL create or replace function trustedsort() returns int as $$ my @arr = (5, 4, 3, 2, 1); my @sorted = sort { elog(NOTICE, "$a $b"); $a <=> $b } @arr; return 1; $$ language 'plperl'; unpatched Safe.pm: select trustedsort(); NOTICE: CONTEXT: PL/Perl function "trustedsort" NOTICE: CONTEXT: PL/Perl function "trustedsort" NOTICE: CONTEXT: PL/Perl function "trustedsort" NOTICE: CONTEXT: PL/Perl function "trustedsort" NOTICE: CONTEXT: PL/Perl function "trustedsort" NOTICE: CONTEXT: PL/Perl function "trustedsort" NOTICE: CONTEXT: PL/Perl function "trustedsort" NOTICE: CONTEXT: PL/Perl function "trustedsort" trustedsort ------------- 1 (1 row) w patched: select trustedsort(); NOTICE: 5 4 CONTEXT: PL/Perl function "trustedsort" NOTICE: 3 2 CONTEXT: PL/Perl function "trustedsort" NOTICE: 4 2 CONTEXT: PL/Perl function "trustedsort" NOTICE: 4 3 CONTEXT: PL/Perl function "trustedsort" NOTICE: 2 1 CONTEXT: PL/Perl function "trustedsort" trustedsort ------------- 1
CC: Tim Bunce <Tim.Bunce [...] pobox.com>, perlbug-followup [...] perl.org, rgs [...] consttype.org
Subject: Re: [perl #60374] Safe.pm sort {} bug with -Dusethreads [PATCH]
Date: Mon, 9 Nov 2009 16:21:47 +0000
To: Alex Hunsaker <badalex [...] gmail.com>
From: Tim Bunce <Tim.Bunce [...] pobox.com>
Download (untitled) / with headers
text/plain 1.1k
On Sun, Nov 08, 2009 at 05:56:21PM -0700, Alex Hunsaker wrote: Show quoted text
> On Sun, Nov 8, 2009 at 16:36, Tim Bunce <Tim.Bunce@pobox.com> wrote:
> > I don't think the "detect if we are returning an anon sub" approach will > > work because the reval() may have compiled some named subs that will be > > invoked later. That's the case with PostgrSQL's embedded plperl, for example.
> > BTW I just tested and it does fix PostgreSQL
Ah, yes, PostgreSQL's embedded plperl does compile anon-subs. Thanks. Also your approach does seem to fix named subs invoked via the anon sub. It's unfortunate that the cost is two extra sub calls to invoke the anon sub :( I've attached a patch to Safe, based on your idea, slightly optimized and generalized, and including some tests. I've tested on a bunch of perl's I have lying around: $ foreachperl makefreshtest [...] ok: /usr/local/perl511-i/bin ok: /usr/local/perl510-pure/bin ok: /usr/local/perl510-thr64/bin ok: /usr/local/perl510/bin ok: /usr/local/perl58-i/bin ok: /usr/local/perl588-64/bin ok: /usr/local/perl588-thr/bin ok: /usr/local/perl589-i/bin ok: /usr/local/perl589/bin Thanks Alex! Tim.

Message body is not shown because sender requested not to inline it.

CC: Alex Hunsaker <badalex [...] gmail.com>, perlbug-followup [...] perl.org, rgs [...] consttype.org
Subject: Re: [perl #60374] Safe.pm sort {} bug with -Dusethreads [PATCH]
Date: Mon, 9 Nov 2009 09:10:18 -0800
To: Tim Bunce <Tim.Bunce [...] pobox.com>
From: "David E. Wheeler" <david [...] kineticode.com>
Download (untitled) / with headers
text/plain 578b
On Nov 9, 2009, at 8:21 AM, Tim Bunce wrote: Show quoted text
> Ah, yes, PostgreSQL's embedded plperl does compile anon-subs. Thanks. > Also your approach does seem to fix named subs invoked via the anon > sub. > It's unfortunate that the cost is two extra sub calls to invoke the > anon sub :( > > I've attached a patch to Safe, based on your idea, slightly optimized > and generalized, and including some tests.
Perhaps it got lost in the shuffle, and I'm certainly glad to see a Safe.pm issue addressed, but is there still an existing issue with sort and -Dusethreads? Best, David
CC: Tim Bunce <Tim.Bunce [...] pobox.com>, Alex Hunsaker <badalex [...] gmail.com>, perlbug-followup [...] perl.org, rgs [...] consttype.org
Subject: Re: [perl #60374] Safe.pm sort {} bug with -Dusethreads [PATCH]
Date: Mon, 9 Nov 2009 20:58:52 +0000
To: "David E. Wheeler" <david [...] kineticode.com>
From: Tim Bunce <Tim.Bunce [...] pobox.com>
Download (untitled) / with headers
text/plain 1023b
On Mon, Nov 09, 2009 at 09:10:18AM -0800, David E. Wheeler wrote: Show quoted text
> On Nov 9, 2009, at 8:21 AM, Tim Bunce wrote: >
>> Ah, yes, PostgreSQL's embedded plperl does compile anon-subs. Thanks. >> Also your approach does seem to fix named subs invoked via the anon sub. >> It's unfortunate that the cost is two extra sub calls to invoke the >> anon sub :( >> >> I've attached a patch to Safe, based on your idea, slightly optimized >> and generalized, and including some tests.
> > Perhaps it got lost in the shuffle,
It reached the ticket ok: http://rt.perl.org/rt3/Ticket/Display.html?id=60374#txn-628757 Show quoted text
> and I'm certainly glad to see a Safe.pm issue addressed, but is there > still an existing issue with sort and -Dusethreads?
In http://rt.perl.org/rt3/Ticket/Display.html?id=60374#txn-627795 Eric Brine says he can't reproduce them, so http://rt.perl.org/rt3/Public/Bug/Display.html?id=37508 http://rt.perl.org/rt3/Public/Bug/Display.html?id=30333 should be closed. (I don't know who gets to do that.) Tim.
CC: Alex Hunsaker <badalex [...] gmail.com>, perlbug-followup [...] perl.org
Subject: Re: [perl #60374] Safe.pm sort {} bug with -Dusethreads
Date: Tue, 10 Nov 2009 14:56:36 +0000
To: Tim Bunce <Tim.Bunce [...] pobox.com>
From: Nicholas Clark <nick [...] ccl4.org>
Download (untitled) / with headers
text/plain 2.2k
On Sun, Nov 08, 2009 at 11:36:46PM +0000, Tim Bunce wrote: Show quoted text
> On Fri, Nov 06, 2009 at 02:30:47PM -0700, Alex Hunsaker wrote:
> > > > Um the last patch I posted should... > > http://rt.perl.org/rt3/Ticket/Display.html?id=60374#txn-612980. And > > while I found it quite ugly and im sure there are better ways to fix > > this... (namely we could fix Opcode::_safe_call_sv to do something > > similar, or fix it as diagnosed in > > http://rt.perl.org/rt3/Ticket/Display.html?id=60374#txn-491794) > > I was more or less looking for some kind of validation as to the > > approach :).
> > I don't think the "detect if we are returning an anon sub" approach will > work because the reval() may have compiled some named subs that will be > invoked later. That's the case with PostgrSQL's embedded plperl, for example. > > http://rt.perl.org/rt3/Public/Bug/Display.html?id=60374#txn-491794 says > "pp_sort uses CopSTASH(PL_curcop) to get the package stash (symbol > table) in order to locate the $a and $b variables in it." > > While pp_sort does call CopSTASH(PL_curcop) directly, all it does with > it is store the value in PL_sortstash which, as far as I can tell, isn't > used or documented anywhere at all. (It could possibly be deleted.) > > The code to lookup the $a and $b uses: > PL_firstgv = gv_fetchpvs("a", GV_ADD|GV_NOTQUAL, SVt_PV); > PL_secondgv = gv_fetchpvs("b", GV_ADD|GV_NOTQUAL, SVt_PV); > and it's Perl_gv_fetchpvn_flags that ends up calling CopSTASH(PL_curcop). > > If there was a way to get the stash pointer (not name) that was in > effect at the time the code was compiled we could use that to lookup $a > and $b.
The logical way to do this would be to indirect via an entry in the subroutine's pad, as the reference would be correctly translated by the ithreads clone code. This probably isn't *trivial* to implement, as a subroutine can contain code compiled in more than one package, but I think* it would be viable at compile time to loop up/store if not there yet, the stash pointer, under the stash's name. Show quoted text
> There are stash pointers, er, stashed in lots of places. > One approach might be CvSTASH(find_runcv(...))
I think that that doesn't cope with package foo; sub bar { # here package is foo package baz; # here package is baz } Nicholas Clark * I think. I don't know this stuff well.
Subject: Re: [perl #60374] Safe.pm sort {} bug with -Dusethreads
Date: Tue, 10 Nov 2009 16:16:20 +0000
To: Tim Bunce <Tim.Bunce [...] pobox.com>, Alex Hunsaker <badalex [...] gmail.com>, perlbug-followup [...] perl.org
From: Tim Bunce <Tim.Bunce [...] pobox.com>
Download (untitled) / with headers
text/plain 2.3k
On Tue, Nov 10, 2009 at 02:56:36PM +0000, Nicholas Clark wrote: Show quoted text
> > > > http://rt.perl.org/rt3/Public/Bug/Display.html?id=60374#txn-491794 says > > "pp_sort uses CopSTASH(PL_curcop) to get the package stash (symbol > > table) in order to locate the $a and $b variables in it." > > > > While pp_sort does call CopSTASH(PL_curcop) directly, all it does with > > it is store the value in PL_sortstash which, as far as I can tell, isn't > > used or documented anywhere at all. (It could possibly be deleted.) > > > > The code to lookup the $a and $b uses: > > PL_firstgv = gv_fetchpvs("a", GV_ADD|GV_NOTQUAL, SVt_PV); > > PL_secondgv = gv_fetchpvs("b", GV_ADD|GV_NOTQUAL, SVt_PV); > > and it's Perl_gv_fetchpvn_flags that ends up calling CopSTASH(PL_curcop). > > > > If there was a way to get the stash pointer (not name) that was in > > effect at the time the code was compiled we could use that to lookup $a > > and $b.
> > The logical way to do this would be to indirect via an entry in the > subroutine's pad, as the reference would be correctly translated by the > ithreads clone code. This probably isn't *trivial* to implement, as a > subroutine can contain code compiled in more than one package, but I think* > it would be viable at compile time to loop up/store if not there yet, the > stash pointer, under the stash's name. >
> > There are stash pointers, er, stashed in lots of places. > > One approach might be CvSTASH(find_runcv(...))
> > I think that that doesn't cope with > > package foo; > sub bar { > # here package is foo > package baz; > # here package is baz > }
I couldn't get it to work even without that (but trying to follow the inside-safe vs outside-safe and compile-time vs run-time intricacies made my rusty old head spin, so I could have done something silly). Alex's approach, of arranging to execute the closure 'inside' the compartment, seems like a good idea from a 'safety' point of view anyway. The fact it works around the bug is a bonus. On the other hand, the fact it costs two extra sub calls is unfortunate. I tried to look for a way to the PL_defstash switch into pp_entersub but couldn't see a viable approach. Show quoted text
> * I think. I don't know this stuff well.
Me neither. It's been many years since I helped develop Opcode and Safe. (I recall refering to Safe as a "failed experiment" more than once.) Tim.
Download (untitled) / with headers
text/plain 200b
I'm not sure who to ask to apply the patch to Safe.pm. I've created a ticket on rt.cpan.org to ensure it doesn't slip though the cracks from that side: https://rt.cpan.org/Ticket/Display.html?id=51574
Now fixed with Safe 2.20.
CC: "Perl5-Porters" <perl5-porters [...] perl.org>
Subject: RE: [perl #60374] Safe.pm sort {} bug with -Dusethreads
Date: Fri, 9 Oct 2009 08:24:34 +0100
To: "demerphq" <demerphq [...] gmail.com>, "Curtis Jewell" <lists.perl.perl5-porters [...] csjewell.fastmail.us>
From: "Steve Hay" <SteveHay [...] planit.com>
Download (untitled) / with headers
text/plain 1.3k
demerphq wrote on 2009-10-08: Show quoted text
> 2009/10/8 Curtis Jewell <lists.perl.perl5-porters@csjewell.fastmail.us>:
>> On Wed, 07 Oct 2009 10:26 -0700, "David E. Wheeler" >> <david@kineticode.com> wrote:
>>> On Oct 7, 2009, at 10:24 AM, Rafael Garcia-Suarez wrote: >>>
>>>> Even if we manage someone to fix threading, this will slow down >>>> sort(). Threading also has a non-negligeable performance impact on >>>> memory and speed of every perl program. >>>> >>>> I think we should advice people in INSTALL to build with threading >>>> only if they really need it, and tell OS vendors to disable threading >>>> on their system perls.
>>> >>> Snow Leopard ships with a threaded Perl. Hell, I think Leopard did. >>> That cat might have left the bag.
>> And Strawberry is threaded.  If I switched it to non-threaded now, I'd >> be stood up to a wall and shot by the Padre people (they require >> threading) among others.
> Im inclined to say that a non-threaded win32 build is pretty close to > broken. >
Why do you say that? I always used to use a non-threaded perl on Win32, and only switched to threaded when I had to because mod_perl-2 needs it. Even now I still don't enable PERL_IMPLICIT_SYS because I have no need for the fork() emulation, and prefer Perl's malloc() (which can't be enabled with PERL_IMPLICIT_SYS) to the CRT's version.


This service is sponsored and maintained by Best Practical Solutions and runs on Perl.org infrastructure.

For issues related to this RT instance (aka "perlbug"), please contact perlbug-admin at perl.org