Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Off-by-one when truncating *shared* array by: $#ary -= $chop_this_many #14151

Closed
p5pRT opened this issue Oct 11, 2014 · 6 comments

Comments

@p5pRT
Copy link
Collaborator

@p5pRT p5pRT commented Oct 11, 2014

Migrated from rt.perl.org#122950 (status was 'resolved')

Searchable as RT122950$

@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented Oct 11, 2014

From andy@hmc.edu

This is a bug report for perl from andy@​hmc.edu,
generated with the help of perlbug 1.39 running under perl 5.18.2.

Off-by-one when truncating *shared* array by​: $#ary -= $chop_this_many

What should happen​: Shared array max index should drop to a new, lower value.

What does happen​: Actual new max index is *too large by one* (Too few elements chopped off)
See the 4th line in the sample output; "...index is​: 5"; It should say "...index is​: 4".

See additional notes and environment details after example code and example output below.

$ ./perl-shared-array-truncate.pl
Before truncation, ordinary array highest index is​: 9
After removing five, ordinary array highest index is​: 4

Before truncation, shared array highest index is​: 9
After removing five, shared array highest index is​: 5

Shared array now contains​: 0 | 1 | 2 | 3 | 4 | 5

$ cat perl-shared-array-truncate.pl
#!/usr/bin/perl -w
use strict;

use threads;
use threads​::shared;

### First with an ordinary array
my @​ordinary_array;
@​ordinary_array = (0,1,2,3,4,5,6,7,8,9);

printf "Before truncation, ordinary array highest index is​: %s\n", $#ordinary_array;
$#ordinary_array = $#ordinary_array - 5;
printf "After removing five, ordinary array highest index is​: %s\n\n", $#ordinary_array;

### Then try it with a shared array
my @​shared_array :shared;
lock @​shared_array; # Same with or without this line
@​shared_array = (0,1,2,3,4,5,6,7,8,9);

printf "Before truncation, shared array highest index is​: %s\n", $#shared_array;
$#shared_array = $#shared_array - 5;
printf "After removing five, shared array highest index is​: %s\n\n", $#shared_array;
printf "Shared array now contains​: %s\n", join(' | ', @​shared_array);

----- environment and notes -----

$ uname -a
Linux andy.xxxxxxx 3.15.10-201.fc20.x86_64 #1 SMP Wed Aug 27 21​:10​:06 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux

$ /usr/bin/perl -v
This is perl 5, version 18, subversion 2 (v5.18.2) built for x86_64-linux-thread-multi
(with 19 registered patches, see perl -V for more detail)

Notes​:

Although I am using the "threads" module in this example script, I am not spawning
any threads here (although I do in the real code in which this problem surfaced).
This is not a concurrency problem. I guess that was obvious. :-)

I am aware that for removing a small number of elements I can "pop" a few times however
I am using this to implement an LRU queue of keys to track the order of elements added
to a hash. I want to occasionally remove ~1000 elements from the end of a ~10000 element
array and I fear that pop on that scale will be slow and inefficient.
Yes, I can compensate for the off-by-one in the meantime.

I am aware that splice is not allowed with shared arrays. This truncation does not appear
to violate (my reading of) the rules for shared arrays, and one of the test scripts at​:

  cpansearch.perl.org/src/RJBS/perl-5.17.6/dist/threads-shared/t/av_simple.t

uses this method to truncate an array.

I am aware of Thread​::Queue.

I have also repeated this on these environments with same results​:

Linux xxxxxxxx 2.6.32-358.2.1.el6.centos.plus.x86_64 #1 SMP Wed Mar 13 02​:09​:07 UTC 2013 x86_64 x86_64 x86_64 GNU/Linux
This is perl, v5.10.1 (*) built for x86_64-linux-thread-multi

Linux andy2 3.11.0-26-generic #45-Ubuntu SMP Tue Jul 15 04​:02​:06 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux
This is perl 5, version 14, subversion 2 (v5.14.2) built for x86_64-linux-gnu-thread-multi

Thank you very much.

Andy


Flags​:
  category=library
  severity=medium
  module=threads​::shared


Site configuration information for perl 5.18.2​:

Configured by Red Hat, Inc. at Tue Jan 7 14​:45​:19 UTC 2014.

Summary of my perl5 (revision 5 version 18 subversion 2) configuration​:
 
  Platform​:
  osname=linux, osvers=3.11.9-200.fc19.x86_64, archname=x86_64-linux-thread-multi
  uname='linux buildvm-12.phx2.fedoraproject.org 3.11.9-200.fc19.x86_64 #1 smp wed nov 20 21​:22​:24 utc 2013 x86_64 x86_64 x86_64 gnulinux '
  config_args='-des -Doptimize=-O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector-strong --param=ssp-buffer-size=4 -grecord-gcc-switches -m64 -mtune=generic -Dccdlflags=-Wl,--enable-new-dtags -Dlddlflags=-shared -O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector-strong --param=ssp-buffer-size=4 -grecord-gcc-switches -m64 -mtune=generic -Wl,-z,relro -Dshrpdir=/usr/lib64 -DDEBUGGING=-g -Dversion=5.18.2 -Dmyhostname=localhost -Dperladmin=root@​localhost -Dcc=gcc -Dcf_by=Red Hat, Inc. -Dprefix=/usr -Dvendorprefix=/usr -Dsiteprefix=/usr/local -Dsitelib=/usr/local/share/perl5 -Dsitearch=/usr/local/lib64/perl5 -Dprivlib=/usr/share/perl5 -Dvendorlib=/usr/share/perl5/vendor_perl -Darchlib=/usr/lib64/perl5 -Dvendorarch=/usr/lib64/perl5/vendor_perl -Darchname=x86_64-linux-thread-multi -Dlibpth=/usr/local/lib64 /lib64 /usr/lib64 -Duseshrplib -Dusethreads -Duseithreads -Dusedtrace=/usr/bin/dtrace -Duselargefiles -Dd_semctl_semun -Di_db -Ui_ndbm -Di_gdbm -Di_shadow -Di_syslog -Dman3ext=3pm -Duseperlio -Dinstallusrbinperl=n -Ubincompat5005 -Uversiononly -Dpager=/usr/bin/less -isr -Dd_gethostent_r_proto -Ud_endhostent_r_proto -Ud_sethostent_r_proto -Ud_endprotoent_r_proto -Ud_setprotoent_r_proto -Ud_endservent_r_proto -Ud_setservent_r_proto -Dscriptdir=/usr/bin -Dusesitecustomize'
  hint=recommended, useposix=true, d_sigaction=define
  useithreads=define, usemultiplicity=define
  useperlio=define, d_sfio=undef, uselargefiles=define, usesocks=undef
  use64bitint=define, use64bitall=define, uselongdouble=undef
  usemymalloc=n, bincompat5005=undef
  Compiler​:
  cc='gcc', ccflags ='-D_REENTRANT -D_GNU_SOURCE -fno-strict-aliasing -pipe -fstack-protector -I/usr/local/include -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64',
  optimize='-O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector-strong --param=ssp-buffer-size=4 -grecord-gcc-switches -m64 -mtune=generic',
  cppflags='-D_REENTRANT -D_GNU_SOURCE -fno-strict-aliasing -pipe -fstack-protector -I/usr/local/include'
  ccversion='', gccversion='4.8.2 20131212 (Red Hat 4.8.2-7)', gccosandvers=''
  intsize=4, longsize=8, ptrsize=8, doublesize=8, byteorder=12345678
  d_longlong=define, longlongsize=8, d_longdbl=define, longdblsize=16
  ivtype='long', ivsize=8, nvtype='double', nvsize=8, Off_t='off_t', lseeksize=8
  alignbytes=8, prototype=define
  Linker and Libraries​:
  ld='gcc', ldflags =' -fstack-protector'
  libpth=/usr/local/lib64 /lib64 /usr/lib64
  libs=-lresolv -lnsl -lgdbm -ldb -ldl -lm -lcrypt -lutil -lpthread -lc -lgdbm_compat
  perllibs=-lresolv -lnsl -ldl -lm -lcrypt -lutil -lpthread -lc
  libc=, so=so, useshrplib=true, libperl=libperl.so
  gnulibc_version='2.18'
  Dynamic Linking​:
  dlsrc=dl_dlopen.xs, dlext=so, d_dlsymun=undef, ccdlflags='-Wl,--enable-new-dtags'
  cccdlflags='-fPIC', lddlflags='-shared -O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector-strong --param=ssp-buffer-size=4 -grecord-gcc-switches -m64 -mtune=generic -Wl,-z,relro '

Locally applied patches​:
  Fedora Patch1​: Removes date check, Fedora/RHEL specific
  Fedora Patch3​: support for libdir64
  Fedora Patch4​: use libresolv instead of libbind
  Fedora Patch5​: USE_MM_LD_RUN_PATH
  Fedora Patch6​: Skip hostname tests, due to builders not being network capable
  Fedora Patch7​: Dont run one io test due to random builder failures
  Fedora Patch9​: Fix find2perl to translate ? glob properly (RT#113054)
  Fedora Patch10​: Update h2ph(1) documentation (RT#117647)
  Fedora Patch11​: Update pod2html(1) documentation (RT#117623)
  Fedora Patch12​: Disable ornaments on perl5db AutoTrace tests (RT#118817)
  Fedora Patch14​: Do not use system Term​::ReadLine​::Gnu in tests (RT#118821)
  Fedora Patch15​: Define SONAME for libperl.so
  Fedora Patch16​: Install libperl.so to -Dshrpdir value
  Fedora Patch18​: Fix crash with \\&$glob_copy (RT#119051)
  Fedora Patch19​: Fix coreamp.t rand test (RT#118237)
  Fedora Patch20​: Reap child in case where exception has been thrown (RT#114722)
  Fedora Patch21​: Fix using regular expressions containing multiple code blocks (RT#117917)
  Fedora Patch200​: Link XS modules to libperl.so with EU​::CBuilder on Linux
  Fedora Patch201​: Link XS modules to libperl.so with EU​::MM on Linux


@​INC for perl 5.18.2​:
  /usr/local/lib64/perl5
  /usr/local/share/perl5
  /usr/lib64/perl5/vendor_perl
  /usr/share/perl5/vendor_perl
  /usr/lib64/perl5
  /usr/share/perl5
  .


Environment for perl 5.18.2​:
  HOME=/home/andy
  LANG=en_US.UTF-8
  LANGUAGE (unset)
  LD_LIBRARY_PATH (unset)
  LOGDIR (unset)
  PATH=/usr/lib64/qt-3.3/bin​:/usr/local/bin​:/usr/bin​:/bin​:/usr/local/sbin​:/usr/sbin​:/home/andy/.local/bin​:/home/andy/bin
  PERL_BADLANG (unset)
  SHELL=/bin/bash

@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented Oct 14, 2014

From @iabyn

On Sat, Oct 11, 2014 at 03​:34​:23AM -0700, via RT wrote​:

Off-by-one when truncating *shared* array by​: $#ary -= $chop_this_many

Thanks for the report. Now fixed in blead by the following two commits​:

commit 399547d
Author​: David Mitchell <davem@​iabyn.com>
AuthorDate​: Mon Oct 13 12​:45​:14 2014 +0100
Commit​: David Mitchell <davem@​iabyn.com>
CommitDate​: Tue Oct 14 12​:33​:06 2014 +0100

  threads​::shared "$#shared = N" off-by-one error
 
  RT #122950
 
  my @​a : shared;
  $#a = 3; # actually set it to 4
 
  There was a simple off-by-one error in the XS code that handled the
  STORESIZE tie method (confusing the array size and fill, which differ
  by 1).
 
  Amazingly, there was no test for it, and no-one had noticed up until now.
 
  Note that this commit causes three tests in object2.t to fail​: this
  is because fixing the $#shared bug exposed another bug that was being
  masked by this one. They will be fixed in the next commit

M dist/threads-shared/lib/threads/shared.pm
M dist/threads-shared/shared.xs
M dist/threads-shared/t/av_simple.t

commit 76eea78
Author​: David Mitchell <davem@​iabyn.com>
AuthorDate​: Tue Oct 14 12​:26​:13 2014 +0100
Commit​: David Mitchell <davem@​iabyn.com>
CommitDate​: Tue Oct 14 12​:33​:07 2014 +0100

  threads​: $#shared = N should destroy
 
  When shrinking a shared array by setting $#shared = N,
  any freed elements should trigger destructors if they are objects,
  but they weren't.
 
  This commit extends the work done by 7d585d2 (which created tmp
  proxys when abandoning elements of arrays and hashes) to the STORESIZE
  method, which is what is triggered by $#a assignment (and indirectly by
  undef @​a).

M dist/threads-shared/shared.xs
M dist/threads-shared/t/object2.t

--
You live and learn (although usually you just live).

@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented Oct 14, 2014

The RT System itself - Status changed from 'new' to 'open'

@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented Oct 15, 2014

@iabyn - Status changed from 'open' to 'pending release'

@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented Jun 2, 2015

From @khwilliamson

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

@p5pRT p5pRT closed this Jun 2, 2015
@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented Jun 2, 2015

@khwilliamson - Status changed from 'pending release' to 'resolved'

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
1 participant
You can’t perform that action at this time.