[perl #75676] PUSH on tied arrays

# New Ticket Created by  Nicholas Clark 
# Please include the string:  [perl #75676]
# in the subject line of all future correspondence about this issue. 
# <URL: http://rt.perl.org/rt3/Ticket/Display.html?id=75676 >


Effectively this is 2 bugs in one ticket, but I can't find a practical way to
split them. Together, they mean that push on a shared array isn't atomic:

$ cat push.pl 
#!perl -w
use strict;
use threads;
use threads::shared;

my @array :shared;

sub hammer {
    for (0..9) {
        my $got = push @array, 'L', 'R';
        print "Got got: $got\n" if $got % 2;
    }
}

my @t;
push @t, threads->new(\&hammer) for 0..19;
$_->join foreach @t;

print join('', @array), "\n";

__END__
$ ./perl -Ilib push.pl 
LRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLR
$ ./perl -Ilib push.pl 
LRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLR
$ ./perl -Ilib push.pl 
LRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLR
$ ./perl -Ilib push.pl 
Got got: 7
Got got: 9
Got got: 11
Got got: 13
Got got: 15
Got got: 17
Got got: 19
Got got: 21
Got got: 23
Got got: 25
Got got: 27
Got got: 29
Got got: 31
Got got: 33
Got got: 35
Got got: 37
Got got: 39
Got got: 41
Got got: 43
Got got: 45
Got got: 47
Got got: 49
Got got: 51
Got got: 53
Got got: 55
Got got: 283
Got got: 285
Got got: 287
Got got: 289
Got got: 291
Got got: 293
Got got: 295
Got got: 297
Got got: 299
Got got: 301
Got got: 303
Got got: 305
Got got: 309
Got got: 311
Got got: 313
Got got: 313
Got got: 315
Got got: 317
Got got: 319
Got got: 321
LRLRLLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLRLR


The return value of push is documented to be the number of elements in the
array. As every push is of two items, that means that after every push
completes the array will have an even number of entries.

Note that

1: Sometimes the script gets an odd number returns from push
2: Sometimes another push starts before the first completes


The problem seems to be twofold

1: threads::shared doesn't hold a lock for the duration of push. (It does for
   unshift.)

I can fix this. However, fixing it would hide the second bug:

2: For push and unshift, the PP code ignores the return value of PUSH and
   UNSHIFT, and makes a (second) call to the tied object, FETCHSIZE.

That feels wrong to me. I have a patch to fix that. (Which is how I stumbled
onto the threads::shared inconsistency)


I believe that fixing all of this is the right thing to do, as currently the
implementation is inconsistent, less efficient than it could be, and prevents
some things from being done at all (such as atomic push and unshift semantics).

However, it does mean changing the exact behaviour of tied arrays from the
emergent behaviour they have had since the second day of their implementation:

http://perl5.git.perl.org/perl.git/commit/a60c0954410db87b # revision 424

[which amends the first attempt here
http://perl5.git.perl.org/perl.git/commit/93965878572d85da # revision 418
]

Nicholas Clark

• [perl #75676] PUSH on tied arrays by Nicholas Clark
• Re: [perl #75676] PUSH on tied arrays by Dave Mitchell
• Re: [perl #75676] PUSH on tied arrays by Dave Mitchell
• Re: [perl #75676] PUSH on tied arrays by Nicholas Clark