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

Owner: Nobody
Requestors: arc <arc [at] cpan.org>
Cc:
AdminCc:

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



Subject: 5.20.x regression in "state" under PERL_NO_COW
Download (untitled) / with headers
text/plain 717b
Niels Larson reports that Perl 5.20 compiled with -DPERL_NO_COW has a regression with state variables: they get reset to undef when accessed: sub no_PERL_NO_COW_regression { state $s; $s = 'foo'; my $c = $s; return defined $s; } ok(no_PERL_NO_COW_regression(), "state variables don't surprisingly disappear when accessed"); Bisection reveals that the bug was introduced in 9ffd39ab75dd662df22fcdafbf7f740838acc898 (between 5.19.6 and 5.19.7), and fixed in c0683843e9299db25f354e2c8c90faa7614950d1 (between 5.21.4 and 5.21.5). I'm creating this ticket so that (a) a regression test can mention it, and (b) the meta-ticket for 5.20.2 can depend on it. -- Aaron Crane ** http://aaroncrane.co.uk/
RT-Send-CC: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 134b
Commit c4a33ecd3009146ea545628e3014a22c637b6bb1 contains a regression test for this bug. -- Aaron Crane ** http://aaroncrane.co.uk/
RT-Send-CC: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 1.6k
On Tue Oct 21 10:36:51 2014, arc wrote: Show quoted text
> Niels Larson reports that Perl 5.20 compiled with -DPERL_NO_COW has a > regression with state variables: they get reset to undef when > accessed: > > sub no_PERL_NO_COW_regression { > state $s; > $s = 'foo'; > my $c = $s; > return defined $s; > } > ok(no_PERL_NO_COW_regression(), > "state variables don't surprisingly disappear when accessed"); > > Bisection reveals that the bug was introduced in > 9ffd39ab75dd662df22fcdafbf7f740838acc898 (between 5.19.6 and 5.19.7), > and fixed in c0683843e9299db25f354e2c8c90faa7614950d1 (between 5.21.4 > and 5.21.5). > > I'm creating this ticket so that (a) a regression test can mention it, > and (b) the meta-ticket for 5.20.2 can depend on it.
With a COW build: $ perl5.20.1 -l -E 'sub { state $s; $s = "foo"x500; my $c = $s; print $s//"undef"}->()' undef SvPADTMP and SvPADSTALE used to share a bit, so (sflags & (SVs_PADTMP|SVf_READONLY|SVf_PROTECT|SVf_IsCOW)) == SVs_PADTMP gave the wrong answer. Putting SVs_PADMY in the list of flags to check will fix the bug for 5.20. However, there is also a problem with the way CHECK_COWBUF_THRESHOLD is defined. Here we use it outside #ifdef PERL_NEW_COPY_ON_WRITE, and when COW is not enabled it is not defined properly. So we need to change the COW constants to be defined regardless of whether COW is enabled, since they are also used for targ swiping. 5.20 without COW is slower than it needs to be, because targ swiping happens far too much, resulting in many malloc calls. I’ll write a separate maint patch on a branch. -- Father Chrysostomos
RT-Send-CC: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 475b
On Tue Oct 21 16:19:36 2014, sprout wrote: Show quoted text
> I’ll write a separate maint patch on a branch.
Please review (and maybe even vote on backporting) the sprout/maint-5.20-123029 branch, which contains two fixes: 1) Don’t swipe short PADTMP buffers when COW is disabled. That was never intended to happen, and it is a regression, in that it causes a significant slowdown. 2) Check the flags properly and don’t steal buffers from state vars. -- Father Chrysostomos
RT-Send-CC: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 298b
This looks well-formed to me for a maint change, though I don't claim to entirely understand that part of the core. With that caveat aside, given that it fixes this regression in 5.20, I vote +1 to backporting it to maint. Thanks, Father Chrysostomos. -- Aaron Crane ** http://aaroncrane.co.uk/
Subject: Re: [perl #123029] 5.20.x regression in "state" under PERL_NO_COW
To: perl5-porters [...] perl.org
From: James E Keenan <jkeen [...] verizon.net>
Date: Wed, 22 Oct 2014 07:16:44 -0400
Download (untitled) / with headers
text/plain 570b
On 10/21/2014 11:28 PM, Father Chrysostomos via RT wrote: Show quoted text
> On Tue Oct 21 16:19:36 2014, sprout wrote:
>> I’ll write a separate maint patch on a branch.
> > Please review (and maybe even vote on backporting) the sprout/maint-5.20-123029 branch, which contains two fixes: >
Is it possible to smoke-test a backport? Show quoted text
> 1) Don’t swipe short PADTMP buffers when COW is disabled. That was > never intended to happen, and it is a regression, in that it > causes a significant slowdown. > 2) Check the flags properly and don’t steal buffers from state vars. >
Subject: Re: [perl #123029] 5.20.x regression in "state" under PERL_NO_COW
Date: Wed, 22 Oct 2014 13:29:59 +0100
From: Aaron Crane <arc [...] cpan.org>
To: James E Keenan via RT <perlbug-followup [...] perl.org>
Download (untitled) / with headers
text/plain 372b
James E Keenan via RT <perlbug-followup@perl.org> wrote: Show quoted text
> Is it possible to smoke-test a backport?
I can't think of a reason why this wouldn't work, to create a smoke-me/jkeenan/maint-123029 branch with the same code as FC's branch: git fetch && git push camel origin/sprout/maint-5.20-123029:smoke-me/jkeenan/maint-123029 -- Aaron Crane ** http://aaroncrane.co.uk/
RT-Send-CC: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 485b
On Wed Oct 22 05:30:41 2014, arc wrote: Show quoted text
> James E Keenan via RT <perlbug-followup@perl.org> wrote:
> > Is it possible to smoke-test a backport?
> > I can't think of a reason why this wouldn't work, to create a > smoke-me/jkeenan/maint-123029 branch with the same code as FC's > branch: > > git fetch && git push camel > origin/sprout/maint-5.20-123029:smoke-me/jkeenan/maint-123029
I’ve already pushed a smoke-me branch. I’m just waiting for results. -- Father Chrysostomos
RT-Send-CC: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 1.1k
On Wed Oct 22 06:03:56 2014, sprout wrote: Show quoted text
> On Wed Oct 22 05:30:41 2014, arc wrote:
> > James E Keenan via RT <perlbug-followup@perl.org> wrote:
> > > Is it possible to smoke-test a backport?
> > > > I can't think of a reason why this wouldn't work, to create a > > smoke-me/jkeenan/maint-123029 branch with the same code as FC's > > branch: > > > > git fetch && git push camel > > origin/sprout/maint-5.20-123029:smoke-me/jkeenan/maint-123029
> > I’ve already pushed a smoke-me branch. I’m just waiting for results.
I’m seeing these failures on Windows on the smoke-me/maint-5.20-123029 branch, but I don’t think they are related: Failures: (common-args) none [default] ../cpan/Module-Build/t/basic.t..............................FAILED Non-zero exit status: 1 Bad plan. You planned 58 tests but ran 47. [default] -DDEBUGGING -Duseithreads ../t/op/threads.t...........................................FAILED Non-zero exit status: 9 Bad plan. You planned 27 tests but ran 9. ../t/win32/popen.t..........................................FAILED Non-zero exit status: 9 No plan found in TAP output -- Father Chrysostomos
Subject: Your ticket against Perl 5 has been resolved
Download (untitled) / with headers
text/plain 222b
Thanks for submitting this ticket The issue should be resolved with the release today of Perl v5.22. If you find that the problem persists, feel free to reopen this ticket -- Karl Williamson for the Perl 5 porters team


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