glob in threads is broken in perl5.18

p5pRT commented 10 years ago

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

Searchable as RT119897$

p5pRT commented 11 years ago

From trast@inf.ethz.ch

It seems \<glob*> contains a race when used in parallel in threads. This test case reliably spews a few errors\, and then segfaults\, when I run it on perl v5.16.2 (stock opensuse 12.3):

----- 8\< ----- #!/usr/bin/perl

use warnings; use strict;

use threads; use threads::shared;

my $nthread = 20;

sub work { foreach (1..10000) { my @files = \<*.supp>; } }

my @threads; foreach my $i (1..$nthread) { push @threads\, threads->create(\&work); }

foreach my $t (@threads) { $t->join(); } ----- >8 -----

There are two kinds of errors shown:

a) Attempt to free unreferenced scalar: SV 0xd1e738\, Perl interpreter: 0xdf8900 at /home/thomas/tmp/perl-thread-glob-test.pl line 13.

b) Thread 1 terminated abnormally: sv_upgrade from type 255 down to type 11 at perl-thread-glob-test.pl line 13.

Followed by a segmentation fault or (rarely) a bus error.

It does not matter whether the glob actually matches any files\, but running in a directory where it does match something greatly increases the frequency of (a) errors\, whereas running in a directory where it doesn't greatly increases the frequency of (b) errors.

The segfault looks like this:

#0 0x0000000000499720 in Perl_hv_common (my_perl=0x8f7e80\, hv=0x889720\, keysv=keysv@entry=0x0\, key=key@entry=0x7fffe8001630 "0"\, klen=\\, flags=0\, action=action@entry=48\, val=val@entry=0x0\, hash=1849449579\, hash@entry=0) at hv.c:638 #1 0x000000000049b57e in Perl_hv_common_key_len (my_perl=my_perl@entry=0x8f7e80\, hv=\\, key=key@entry=0x7fffe8001630 "0"\, klen_i32=\\, action=action@entry=48\, val=val@entry=0x0\, hash=hash@entry=0) at hv.c:333 #2 0x00007ffff68b55d5 in iterate (my_perl=0x8f7e80\, globber=0x7ffff68b5c60 \<csh_glob>) at Glob.xs:84 #3 0x00000000004e7225 in Perl_pp_glob (my_perl=0x8f7e80) at pp_sys.c:379 #4 0x000000000049ee16 in Perl_runops_standard (my_perl=0x8f7e80) at run.c:41 #5 0x0000000000436deb in Perl_call_sv (my_perl=0x8f7e80\, sv=0x9baba0\, flags=11) at perl.c:2705 #6 0x00007ffff6ece0c9 in ?? () from /usr/lib/perl5/5.16.2/x86_64-linux-thread-multi/auto/threads/threads.so #7 0x00007ffff7488e0f in start_thread () from /lib64/libpthread.so.0 #8 0x00007ffff71bc7dd in clone () from /lib64/libc.so.6

or sometimes just starts at the iterate() frame:

#0 0x00007ffff68b569b in iterate (my_perl=0xcbad60\, globber=0x7ffff68b5c60 \<csh_glob>) at Glob.xs:96 #1 0x00000000004e7225 in Perl_pp_glob (my_perl=0xcbad60) at pp_sys.c:379 #2 0x000000000049ee16 in Perl_runops_standard (my_perl=0xcbad60) at run.c:41 #3 0x0000000000436deb in Perl_call_sv (my_perl=0xcbad60\, sv=0xd7c690\, flags=11) at perl.c:2705 #4 0x00007ffff6ece0c9 in ?? () from /usr/lib/perl5/5.16.2/x86_64-linux-thread-multi/auto/threads/threads.so #5 0x00007ffff7488e0f in start_thread () from /lib64/libpthread.so.0 #6 0x00007ffff71bc7dd in clone () from /lib64/libc.so.6

vincent on #perl ran the test script on many perl versions (thanks!) and reported this result:

$ for p in ~/perl/builds/bin/perl5*thr* ; do echo $p ; $p x.pl ; done /home/vince/perl/builds/bin/perl5.10.0-dbg-psn-shr-thr-64 /home/vince/perl/builds/bin/perl5.10.1-dbg-psn-shr-thr-64 /home/vince/perl/builds/bin/perl5.12.0-dbg-psn-shr-thr-64 Segmentation fault /home/vince/perl/builds/bin/perl5.12.1-dbg-psn-shr-thr-64 Segmentation fault /home/vince/perl/builds/bin/perl5.12.2-dbg-psn-shr-thr-64 Segmentation fault /home/vince/perl/builds/bin/perl5.12.3-dbg-psn-shr-thr-64 Segmentation fault /home/vince/perl/builds/bin/perl5.12.4-dbg-psn-shr-thr-64 Segmentation fault /home/vince/perl/builds/bin/perl5.12.5-dbg-psn-shr-thr-64 Segmentation fault /home/vince/perl/builds/bin/perl5.14.0-dbg-psn-shr-thr-64 /home/vince/perl/builds/bin/perl5.14.1-dbg-psn-shr-thr-64 /home/vince/perl/builds/bin/perl5.14.2-dbg-psn-shr-thr-64 /home/vince/perl/builds/bin/perl5.14.3-dbg-psn-shr-thr-64 /home/vince/perl/builds/bin/perl5.16.0-dbg-psn-shr-thr-64 perl5.16.0-dbg-psn-shr-thr-64: Glob.xs:232: csh_glob: Assertion `((svtype)((entries)->sv_flags & 0xff)) != SVt_PVAV' failed. Aborted /home/vince/perl/builds/bin/perl5.16.1-dbg-psn-shr-thr-64 perl5.16.1-dbg-psn-shr-thr-64: Glob.xs:232: csh_glob: Assertion `((svtype)((entries)->sv_flags & 0xff)) != SVt_PVAV' failed. Aborted /home/vince/perl/builds/bin/perl5.16.2-dbg-psn-shr-thr-64 perl5.16.2-dbg-psn-shr-thr-64: Glob.xs:232: csh_glob: Assertion `((svtype)((entries)->sv_flags & 0xff)) != SVt_PVAV' failed. perl5.16.2-dbg-psn-shr-thr-64: Glob.xs:232: csh_glob: Assertion `((svtype)((entries)->sv_flags & 0xff)) != SVt_PVAV' failed. Aborted /home/vince/perl/builds/bin/perl5.17.3-dbg-psn-shr-thr-64 perl5.17.3-dbg-psn-shr-thr-64: Glob.xs:232: csh_glob: Assertion `((svtype)((entries)->sv_flags & 0xff)) != SVt_PVAV' failed. Aborted /home/vince/perl/builds/bin/perl5.17.6-dbg-psn-shr-thr-64 Segmentation fault /home/vince/perl/builds/bin/perl5.17.7-dbg-psn-shr-thr-64 Attempt to free nonexistent shared string ' ��'\, Perl interpreter: 0x1a4c5c0 at x.pl line 13. Segmentation fault /home/vince/perl/builds/bin/perl5.8.0-dbg-shr-thr-64 /home/vince/perl/builds/bin/perl5.8.1-dbg-shr-thr-64 /home/vince/perl/builds/bin/perl5.8.2-dbg-shr-thr-64 /home/vince/perl/builds/bin/perl5.8.3-dbg-shr-thr-64 /home/vince/perl/builds/bin/perl5.8.4-dbg-shr-thr-64 /home/vince/perl/builds/bin/perl5.8.5-dbg-shr-thr-64 /home/vince/perl/builds/bin/perl5.8.6-dbg-shr-thr-64 /home/vince/perl/builds/bin/perl5.8.7-dbg-shr-thr-64 /home/vince/perl/builds/bin/perl5.8.8-dbg-shr-thr-64 /home/vince/perl/builds/bin/perl5.8.9-dbg-psn-shr-thr-64

--- perlbug -d ---

Flags: category=core severity=low

This perlbug was built using Perl 5.16.2 - Mon Mar 11 11:01:56 UTC 2013 It is being executed now by Perl 5.16.2 - Mon Mar 11 10:54:42 UTC 2013.

Site configuration information for perl 5.16.2:

Configured by abuild at Mon Mar 11 10:54:42 UTC 2013.

Summary of my perl5 (revision 5 version 16 subversion 2) configuration:
Platform: osname=linux\, osvers=3.4.6-2.10-xen\, archname=x86_64-linux-thread-multi uname='linux build05 3.4.6-2.10-xen #1 smp thu jul 26 09:36:26 utc 2012 (641c197) x86_64 x86_64 x86_64 gnulinux ' config_args='-ds -e -Dprefix=/usr -Dvendorprefix=/usr -Dinstallusrbinperl -Dusethreads -Di_db -Di_dbm -Di_ndbm -Di_gdbm -Dd_dbm_open -Duseshrplib=true -Doptimize=-fmessage-length=0 -O2 -Wall -D_FORTIFY_SOURCE=2 -fstack-protector -funwind-tables -fasynchronous-unwind-tables -g -Wall -pipe -Accflags=-DPERL_USE_SAFE_PUTENV -Dotherlibdirs=/usr/lib/perl5/site_perl' 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='cc'\, ccflags ='-D_REENTRANT -D_GNU_SOURCE -DPERL_USE_SAFE_PUTENV -fno-strict-aliasing -pipe -fstack-protector -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64'\, optimize='-fmessage-length=0 -O2 -Wall -D_FORTIFY_SOURCE=2 -fstack-protector -funwind-tables -fasynchronous-unwind-tables -g -Wall -pipe'\, cppflags='-D_REENTRANT -D_GNU_SOURCE -DPERL_USE_SAFE_PUTENV -fno-strict-aliasing -pipe -fstack-protector' ccversion=''\, gccversion='4.7.2 20130108 [gcc-4_7-branch revision 195012]'\, 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='cc'\, ldflags =' -L/usr/local/lib64 -fstack-protector' libpth=/lib64 /usr/lib64 /usr/local/lib64 libs=-lm -ldl -lcrypt -lpthread perllibs=-lm -ldl -lcrypt -lpthread libc=/lib64/libc-2.17.so\, so=so\, useshrplib=true\, libperl=libperl.so gnulibc_version='2.17' Dynamic Linking: dlsrc=dl_dlopen.xs\, dlext=so\, d_dlsymun=undef\, ccdlflags='-Wl\,-E -Wl\,-rpath\,/usr/lib/perl5/5.16.2/x86_64-linux-thread-multi/CORE' cccdlflags='-fPIC'\, lddlflags='-shared -L/usr/local/lib64 -fstack-protector'

Locally applied patches:

@INC for perl 5.16.2: /usr/lib/perl5/site_perl/5.16.2/x86_64-linux-thread-multi /usr/lib/perl5/site_perl/5.16.2 /usr/lib/perl5/vendor_perl/5.16.2/x86_64-linux-thread-multi /usr/lib/perl5/vendor_perl/5.16.2 /usr/lib/perl5/5.16.2/x86_64-linux-thread-multi /usr/lib/perl5/5.16.2 /usr/lib/perl5/site_perl/5.16.2/x86_64-linux-thread-multi /usr/lib/perl5/site_perl/5.16.2 /usr/lib/perl5/site_perl .

Environment for perl 5.16.2: HOME=/home/thomas LANG=en_US.UTF-8 LANGUAGE= LC_NUMERIC=POSIX LD_LIBRARY_PATH=/usr/lib64/mpi/gcc/openmpi/lib64:/home/thomas/.local/lib:/home/thomas/.local/lib64:/home/thomas/.local/lib64/nest LOGDIR (unset) PATH=/home/thomas/.local/bin:/usr/lib64/mpi/gcc/openmpi/bin:/home/thomas/bin:/usr/local/bin:/usr/bin:/bin:/usr/bin/X11:/usr/X11R6/bin:/usr/games:/home/thomas/.local/bin:/usr/local/sbin:/usr/sbin:/sbin:/usr/games PERL_BADLANG (unset) SHELL=/bin/bash

-- Thomas Rast trast@{inf\,student}.ethz.ch

p5pRT commented 11 years ago

From @cpansprout

On Wed May 01 04:55:31 2013\, trast@inf.ethz.ch wrote:

It seems \<glob*> contains a race when used in parallel in threads. This test case reliably spews a few errors\, and then segfaults\, when I run it on perl v5.16.2 (stock opensuse 12.3):

----- 8\< ----- #!/usr/bin/perl

use warnings; use strict;

use threads; use threads::shared;

my $nthread = 20;

sub work { foreach (1..10000) { my @files = \<*.supp>; } }

my @threads; foreach my $i (1..$nthread) { push @threads\, threads->create(\&work); }

foreach my $t (@threads) { $t->join(); } ----- >8 -----

There are two kinds of errors shown:

a) Attempt to free unreferenced scalar: SV 0xd1e738\, Perl interpreter: 0xdf8900 at /home/thomas/tmp/perl-thread-glob-test.pl line 13.

b) Thread 1 terminated abnormally: sv_upgrade from type 255 down to type 11 at perl-thread-glob-test.pl line 13.

Followed by a segmentation fault or (rarely) a bus error.

I can produce this bug on Mac OS X as well.

I also see ‘Attempt to free nonexistent shared string’\, ‘Assertion failed’\, ‘panic: free from wrong pool’. The errors I get are different in each run.

I have tried to diagnose this\, but I am stumped.

It has nothing to do with readdir. I get the same thing with \<{a}>\, but \<{a\,b}{a\,b}> triggers it more reliably.

The bug can’t be in glob_expand. I changed glob_expand to return early\, and the memory corruption persists.

But I can’t see anything wrong with the code. Nothing uses globals.

--

Father Chrysostomos

p5pRT commented 11 years ago

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

p5pRT commented 11 years ago

From @cpansprout

On Sat Jul 06 17:53:30 2013\, sprout wrote:

The bug can’t be in glob_expand.

I meant globextend.

--

Father Chrysostomos

p5pRT commented 10 years ago

From @nthykier

Created by @nthykier

Dear perl porters\,

When Debian migrated to Perl5.18.1\, I started to experience random seg. faults in one of my perl scripts. hugmeir from #p5p devised the following mimimal test case:

""" use threads; scalar glob("*"); threads->create(sub { glob("*") })->join(); """

The test case itself does not crash\, but it does spew out warnings like:

""" Unbalanced string table refcount: (1) for "I" during global destruction. Attempt to free nonexistent shared string 'I'\, [...] Attempt to free unreferenced scalar: SV 0x8c6a1fc\, [...] """

Based on that\, I replaced my calls to "glob" in the threads and my script stopped crashing.

hugmeir also suggested that he believed this bug to also affect blead (I have not tested that assertion). TonyC from #p5p suggested that it might be because the "glob state isn't being cloned".

~Niels

Perl Info

``` Flags: category=core severity=medium Site configuration information for perl 5.18.1: Configured by Debian Project at Sun Sep 1 00:18:57 UTC 2013. Summary of my perl5 (revision 5 version 18 subversion 1) configuration: Platform: osname=linux, osvers=3.2.0-4-amd64, archname=i486-linux-gnu-thread-multi-64int uname='linux binet 3.2.0-4-amd64 #1 smp debian 3.2.46-1+deb7u1 i686 gnulinux ' config_args='-Dusethreads -Duselargefiles -Dccflags=-DDEBIAN -D_FORTIFY_SOURCE=2 -g -O2 -fstack-protector --param=ssp-buffer-size=4 -Wformat -Werror=format-security -Dldflags= -Wl,-z,relro -Dlddlflags=-shared -Wl,-z,relro -Dcccdlflags=-fPIC -Darchname=i486-linux-gnu -Dprefix=/usr -Dprivlib=/usr/share/perl/5.18 -Darchlib=/usr/lib/perl/5.18 -Dvendorprefix=/usr -Dvendorlib=/usr/share/perl5 -Dvendorarch=/usr/lib/perl5 -Dsiteprefix=/usr/local -Dsitelib=/usr/local/share/perl/5.18.1 -Dsitearch=/usr/local/lib/perl/5.18.1 -Dman1dir=/usr/share/man/man1 -Dman3dir=/usr/share/man/man3 -Dsiteman1dir=/usr/local/man/man1 -Dsiteman3dir=/usr/local/man/man3 -Duse64bitint -Dman1ext=1 -Dman3ext=3perl -Dpager=/usr/bin/sensible-pager -Uafs -Ud_csh -Ud_ualarm -Uusesfio -Uusenm -Ui_libutil -Uversiononly -DDEBUGGING=-g -Doptimize=-O2 -Duseshrplib -Dlibperl=libperl.so.5.18.1 -des' hint=recommended, useposix=true, d_sigaction=define useithreads=define, usemultiplicity=define useperlio=define, d_sfio=undef, uselargefiles=define, usesocks=undef use64bitint=define, use64bitall=undef, uselongdouble=undef usemymalloc=n, bincompat5005=undef Compiler: cc='cc', ccflags ='-D_REENTRANT -D_GNU_SOURCE -DDEBIAN -fstack-protector -fno-strict-aliasing -pipe -I/usr/local/include -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64', optimize='-O2 -g', cppflags='-D_REENTRANT -D_GNU_SOURCE -DDEBIAN -fstack-protector -fno-strict-aliasing -pipe -I/usr/local/include' ccversion='', gccversion='4.8.1', gccosandvers='' intsize=4, longsize=4, ptrsize=4, doublesize=8, byteorder=12345678 d_longlong=define, longlongsize=8, d_longdbl=define, longdblsize=12 ivtype='long long', ivsize=8, nvtype='double', nvsize=8, Off_t='off_t', lseeksize=8 alignbytes=4, prototype=define Linker and Libraries: ld='cc', ldflags =' -fstack-protector -L/usr/local/lib' libpth=/usr/local/lib /lib/i386-linux-gnu /lib/../lib /usr/lib/i386-linux-gnu /usr/lib/../lib /lib /usr/lib libs=-lgdbm -lgdbm_compat -ldb -ldl -lm -lpthread -lc -lcrypt perllibs=-ldl -lm -lpthread -lc -lcrypt libc=, so=so, useshrplib=true, libperl=libperl.so.5.18.1 gnulibc_version='2.17' Dynamic Linking: dlsrc=dl_dlopen.xs, dlext=so, d_dlsymun=undef, ccdlflags='-Wl,-E' cccdlflags='-fPIC', lddlflags='-shared -L/usr/local/lib -fstack-protector' Locally applied patches: DEBPKG:debian/cpan_definstalldirs - Provide a sensible INSTALLDIRS default for modules installed from CPAN. DEBPKG:debian/db_file_ver - http://bugs.debian.org/340047 Remove overly restrictive DB_File version check. DEBPKG:debian/doc_info - Replace generic man(1) instructions with Debian-specific information. DEBPKG:debian/enc2xs_inc - http://bugs.debian.org/290336 Tweak enc2xs to follow symlinks and ignore missing @INC directories. DEBPKG:debian/errno_ver - http://bugs.debian.org/343351 Remove Errno version check due to upgrade problems with long-running processes. DEBPKG:debian/libperl_embed_doc - http://bugs.debian.org/186778 Note that libperl-dev package is required for embedded linking DEBPKG:fixes/respect_umask - Respect umask during installation DEBPKG:debian/writable_site_dirs - Set umask approproately for site install directories DEBPKG:debian/extutils_set_libperl_path - EU:MM: Set location of libperl.a to /usr/lib DEBPKG:debian/no_packlist_perllocal - Don't install .packlist or perllocal.pod for perl or vendor DEBPKG:debian/prefix_changes - Fiddle with *PREFIX and variables written to the makefile DEBPKG:debian/fakeroot - Postpone LD_LIBRARY_PATH evaluation to the binary targets. DEBPKG:debian/instmodsh_doc - Debian policy doesn't install .packlist files for core or vendor. DEBPKG:debian/ld_run_path - Remove standard libs from LD_RUN_PATH as per Debian policy. DEBPKG:debian/libnet_config_path - Set location of libnet.cfg to /etc/perl/Net as /usr may not be writable. DEBPKG:debian/mod_paths - Tweak @INC ordering for Debian DEBPKG:debian/module_build_man_extensions - http://bugs.debian.org/479460 Adjust Module::Build manual page extensions for the Debian Perl policy DEBPKG:debian/prune_libs - http://bugs.debian.org/128355 Prune the list of libraries wanted to what we actually need. DEBPKG:fixes/net_smtp_docs - [rt.cpan.org #36038] http://bugs.debian.org/100195 Document the Net::SMTP 'Port' option DEBPKG:debian/perlivp - http://bugs.debian.org/510895 Make perlivp skip include directories in /usr/local DEBPKG:debian/cpanplus_definstalldirs - http://bugs.debian.org/533707 Configure CPANPLUS to use the site directories by default. DEBPKG:debian/cpanplus_config_path - Save local versions of CPANPLUS::Config::System into /etc/perl. DEBPKG:debian/deprecate-with-apt - http://bugs.debian.org/702096 Point users to Debian packages of deprecated core modules DEBPKG:debian/squelch-locale-warnings - http://bugs.debian.org/508764 Squelch locale warnings in Debian package maintainer scripts DEBPKG:debian/skip-upstream-git-tests - Skip tests specific to the upstream Git repository DEBPKG:debian/patchlevel - http://bugs.debian.org/567489 List packaged patches for 5.18.1-3 in patchlevel.h DEBPKG:debian/skip-kfreebsd-crash - http://bugs.debian.org/628493 [perl #96272] Skip a crashing test case in t/op/threads.t on GNU/kFreeBSD DEBPKG:fixes/document_makemaker_ccflags - http://bugs.debian.org/628522 [rt.cpan.org #68613] Document that CCFLAGS should include $Config{ccflags} DEBPKG:debian/find_html2text - http://bugs.debian.org/640479 Configure CPAN::Distribution with correct name of html2text DEBPKG:debian/hurd_test_todo_syslog - http://bugs.debian.org/650093 Disable failing GNU/Hurd tests in cpan/Sys-Syslog/t/syslog.t DEBPKG:debian/hurd_test_skip_sigdispatch - http://bugs.debian.org/650188 Disable failing GNU/Hurd tests op/sigdispatch.t DEBPKG:debian/hurd_test_skip_stack - http://bugs.debian.org/650175 Disable failing GNU/Hurd tests dist/threads/t/stack.t DEBPKG:debian/hurd_test_skip_pipe - http://bugs.debian.org/650187 Disable failing GNU/Hurd tests io/pipe.t DEBPKG:debian/hurd_test_skip_io_pipe - http://bugs.debian.org/650096 Disable failing GNU/Hurd tests dist/IO/t/io_pipe.t DEBPKG:fixes/manpage_name_Test-Harness - http://bugs.debian.org/650451 [rt.cpan.org #73399] cpan/Test-Harness: add NAME headings in modules with POD DEBPKG:debian/makemaker-pasthru - http://bugs.debian.org/660195 [rt.cpan.org #28632] Make EU::MM pass LD through to recursive Makefile.PL invocations DEBPKG:debian/perl5db-x-terminal-emulator.patch - http://bugs.debian.org/668490 Invoke x-terminal-emulator rather than xterm in perl5db.pl DEBPKG:debian/cpan-missing-site-dirs - http://bugs.debian.org/688842 Fix CPAN::FirstTime defaults with nonexisting site dirs if a parent is writable DEBPKG:debian/hurd_net_ping_disable_test - http://bugs.debian.org/709385 Disable failing Net-Ping tests for GNU/Hurd DEBPKG:fixes/memoize_storable_nstore - [rt.cpan.org #77790] http://bugs.debian.org/587650 Memoize::Storable: respect 'nstore' option not respected DEBPKG:fixes/net_ftp_failed_command - [rt.cpan.org #37700] http://bugs.debian.org/491062 Net::FTP: cope gracefully with a failed command DEBPKG:fixes/perlbug-patchlist - [3541c11] http://bugs.debian.org/710842 [perl #118433] Make perlbug look up the list of local patches at run time DEBPKG:fixes/regexp-preserve - http://bugs.debian.org/718209 [perl #118213] [f4194b2] RT #118213: handle $r=qr/.../; /$r/p properly DEBPKG:fixes/regexp-preserve-testcases - http://bugs.debian.org/718209 [perl #118213] [4d7b2f5] Disable new //p tests DEBPKG:fixes/module_metadata_security_doc - [68cdd4b] CVE-2013-1437 documentation fix @INC for perl 5.18.1: /etc/perl /usr/local/lib/perl/5.18.1 /usr/local/share/perl/5.18.1 /usr/lib/perl5 /usr/share/perl5 /usr/lib/perl/5.18 /usr/share/perl/5.18 /usr/local/lib/site_perl . Environment for perl 5.18.1: LANG=en_DK.UTF-8 LANGUAGE (unset) LD_LIBRARY_PATH (unset) LOGDIR (unset) PATH=/usr/local/bin:/usr/bin:/bin:/usr/games PERL_BADLANG (unset) SHELL=/bin/bash ```

p5pRT commented 10 years ago

From @cpansprout

On Fri Sep 20 04:09:25 2013\, niels@thykier.net wrote:

This is a bug report for perl from niels@thykier.net\, generated with the help of perlbug 1.39 running under perl 5.18.1.

----------------------------------------------------------------- [Please describe your issue here]

Dear perl porters\,

When Debian migrated to Perl5.18.1\, I started to experience random seg. faults in one of my perl scripts. hugmeir from #p5p devised the following mimimal test case:

""" use threads; scalar glob("*"); threads->create(sub { glob("*") })->join(); """

The test case itself does not crash\, but it does spew out warnings like:

""" Unbalanced string table refcount: (1) for "I" during global destruction. Attempt to free nonexistent shared string 'I'\, [...] Attempt to free unreferenced scalar: SV 0x8c6a1fc\, [...] """

Based on that\, I replaced my calls to "glob" in the threads and my script stopped crashing.

hugmeir also suggested that he believed this bug to also affect blead (I have not tested that assertion). TonyC from #p5p suggested that it might be because the "glob state isn't being cloned".

Interesting. Could this be the same as #117823?

--

Father Chrysostomos

p5pRT commented 10 years ago

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

p5pRT commented 10 years ago

From @nthykier

On 2013-09-20 17:39\, Father Chrysostomos via RT wrote:

[...]

Interesting. Could this be the same as #117823?

To be honest\, I am not sure. When I discovered the issue\, I ran a gdb on it produce a stack trace (see attached file). From what I can tell\, the stack traces are different (mine involves "op-freeing" vs the #117823 seems to deal with pp_glob)[0]. That said\, the symptoms and cause look similar[1]. So\, I would not be surprised if a fix for one of these bugs also fixes the other.

~Niels

[0] An extra data point is the assertion triggered. In the Debian bug #723805[DebBug]\, Niko Tyni reported that he could trigger the following assertion reliably.

debugperl: hv.c:356: Perl_hv_common: Assertion `((svtype)((hv)->sv_flags & 0xff)) == SVt_PVHV' failed.

But RT#117823 lists:

Glob.xs:232: csh_glob: Assertion `((svtype)((entries)->sv_flags & 0xff)) != SVt_PVAV' failed.

[1] E.g. both bugs involves calling "glob" from threads. My original script also "rarely" (i.e. "once") triggered a bus error rather than a seg. fault.

[DebBug] http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=723805

p5pRT commented 10 years ago

From @nthykier

gdb-bt.log

p5pRT commented 10 years ago

From @Hugmeir

On Fri\, Sep 20\, 2013 at 12:39 PM\, Father Chrysostomos via RT \< perlbug-followup@perl.org> wrote:

On Fri Sep 20 04:09:25 2013\, niels@thykier.net wrote:

This is a bug report for perl from niels@thykier.net\, generated with the help of perlbug 1.39 running under perl 5.18.1.

----------------------------------------------------------------- [Please describe your issue here]

Dear perl porters\,

When Debian migrated to Perl5.18.1\, I started to experience random seg. faults in one of my perl scripts. hugmeir from #p5p devised the following mimimal test case:

""" use threads; scalar glob("*"); threads->create(sub { glob("*") })->join(); """

The test case itself does not crash\, but it does spew out warnings like:

""" Unbalanced string table refcount: (1) for "I" during global destruction. Attempt to free nonexistent shared string 'I'\, [...] Attempt to free unreferenced scalar: SV 0x8c6a1fc\, [...] """

Based on that\, I replaced my calls to "glob" in the threads and my script stopped crashing.

hugmeir also suggested that he believed this bug to also affect blead (I have not tested that assertion). TonyC from #p5p suggested that it might be because the "glob state isn't being cloned".

Interesting. Could this be the same as #117823?

Yes. The problem for both is that x_GLOB_ENTRIES should be thread-local\, but is unintentionally being shared between threads.

This patch solves the issue:

Inline Patch

```diff diff --git a/ext/File-Glob/Glob.xs b/ext/File-Glob/Glob.xs index b3705b3..181dbbc 100644 --- a/ext/File-Glob/Glob.xs +++ b/ext/File-Glob/Glob.xs @@ -396,6 +396,13 @@ PPCODE: iterate(aTHX_ doglob_iter_wrapper); SPAGAIN; +void +CLONE(...) +CODE: + PERL_UNUSED_ARG(items); + MY_CXT_CLONE; + MY_CXT.x_GLOB_ENTRIES = NULL; + BOOT: { #ifndef PERL_EXTERNAL_GLOB ```

But it means that this:

./perl -Ilib -Mthreads -E 'sub foo {scalar glob("*")} foo(); say threads->create(\&foo)->join() for 1..3'

Will return "Artistic" four times\, rather than "Artristic" followed by the second file thrice. If the latter is the desired behavior\, then we need to hv_dup(x_GLOB_ENTRIES) in CLONE() as well.

p5pRT commented 10 years ago

From @cpansprout

On Fri Sep 20 16:31:00 2013\, Hugmeir wrote:

Yes. The problem for both is that x_GLOB_ENTRIES should be thread-local\, but is unintentionally being shared between threads.

This patch solves the issue:

diff --git a/ext/File-Glob/Glob.xs b/ext/File-Glob/Glob.xs index b3705b3..181dbbc 100644 --- a/ext/File-Glob/Glob.xs +++ b/ext/File-Glob/Glob.xs @@ -396\,6 +396\,13 @@ PPCODE: iterate(aTHX_ doglob_iter_wrapper); SPAGAIN;

+void +CLONE(...) +CODE: + PERL_UNUSED_ARG(items); + MY_CXT_CLONE; + MY_CXT.x_GLOB_ENTRIES = NULL; + BOOT: { #ifndef PERL_EXTERNAL_GLOB

But it means that this:

./perl -Ilib -Mthreads -E 'sub foo {scalar glob("*")} foo(); say threads->create(\&foo)->join() for 1..3'

Will return "Artistic" four times\, rather than "Artristic" followed by the second file thrice. If the latter is the desired behavior\, then we need to hv_dup(x_GLOB_ENTRIES) in CLONE() as well.

Yes\, the latter. That would be backward-compatible with when File::Glob still worked.

Thank you for tackling this. This is entirely my fault\, as I was the one who moved it from Perl space to XS.

--

Father Chrysostomos

p5pRT commented 10 years ago

From @Hugmeir

On Fri\, Sep 20\, 2013 at 9:16 PM\, Father Chrysostomos via RT \< perlbug-followup@perl.org> wrote:

On Fri Sep 20 16:31:00 2013\, Hugmeir wrote:

Yes. The problem for both is that x_GLOB_ENTRIES should be thread-local\, but is unintentionally being shared between threads.

This patch solves the issue:

diff --git a/ext/File-Glob/Glob.xs b/ext/File-Glob/Glob.xs index b3705b3..181dbbc 100644 --- a/ext/File-Glob/Glob.xs +++ b/ext/File-Glob/Glob.xs @@ -396\,6 +396\,13 @@ PPCODE: iterate(aTHX_ doglob_iter_wrapper); SPAGAIN;

+void +CLONE(...) +CODE: + PERL_UNUSED_ARG(items); + MY_CXT_CLONE; + MY_CXT.x_GLOB_ENTRIES = NULL; + BOOT: { #ifndef PERL_EXTERNAL_GLOB

But it means that this:

./perl -Ilib -Mthreads -E 'sub foo {scalar glob("*")} foo(); say threads->create(\&foo)->join() for 1..3'

Will return "Artistic" four times\, rather than "Artristic" followed by the second file thrice. If the latter is the desired behavior\, then we need to hv_dup(x_GLOB_ENTRIES) in CLONE() as well.

Yes\, the latter. That would be backward-compatible with when File::Glob still worked.

I'm afraid I don't know how to implement this properly! Unlike dup magic\, CLONE() doesn't get a CLONE_PARAMS argument\, so calling sv_dup() and friends requires manual intervention\, and I don't believe that the correct procedure is documented anywhere. The only place in the core that does an sv_dup() during CLONE() is threads.xs\, and that's likely not a good role model for more normal code; meanwhile\, the paradigm in the few CPAN modules that do this[*] is "grab the interpreter during BOOT\, use that to construct a fake CLONE_PARAMS\, and then replace the interpreter with aTHX after MY_CXT_CLONE" Which\, on top of the previous patch\, becomes this:

Inline Patch

```diff diff --git a/ext/File-Glob/Glob.xs b/ext/File-Glob/Glob.xs index 181dbbc..118d88e 100644 --- a/ext/File-Glob/Glob.xs +++ b/ext/File-Glob/Glob.xs @@ -9,6 +9,9 @@ #define MY_CXT_KEY "File::Glob::_guts" XS_VERSION typedef struct { +#ifdef USE_ITHREADS + tTHX interp; +#endif int x_GLOB_ERROR; HV * x_GLOB_ENTRIES; Perl_ophook_t x_GLOB_OLD_OPHOOK; @@ -396,12 +399,32 @@ PPCODE: iterate(aTHX_ doglob_iter_wrapper); SPAGAIN; +#ifdef USE_ITHREADS + void CLONE(...) +INIT: + HV *glob_entries_clone = NULL; CODE: PERL_UNUSED_ARG(items); - MY_CXT_CLONE; - MY_CXT.x_GLOB_ENTRIES = NULL; + { + dMY_CXT; + if ( MY_CXT.x_GLOB_ENTRIES ) { + CLONE_PARAMS param; + param.stashes = NULL; + param.flags = 0; + param.proto_perl = MY_CXT.interp; + + glob_entries_clone = ```

MUTABLE_HV(sv_dup_inc((SV*)MY_CXT.x_GLOB_ENTRIES, &param)); + } + } + { + MY_CXT_CLONE; + MY_CXT.x_GLOB_ENTRIES = glob_entries_clone; + MY_CXT.interp = aTHX; + } + +#endif

BOOT: { @@ -418\,6 +441\,9 @@ BOOT: dMY_CXT; MY_CXT.x_GLOB_ENTRIES = NULL; MY_CXT.x_GLOB_OLD_OPHOOK = PL_opfreehook; +#ifdef USE_ITHREADS + MY_CXT.interp = aTHX; +#endif PL_opfreehook = glob_ophook; } }

And with that: ./perl -Ilib -Mthreads -E 'sub foo {scalar glob("*")} say foo(); say "\t"\, threads->create(\&foo)->join() for 1..3; say ""; say "\t\t"\, threads->create(sub { say "\t"\, foo(); threads->create(\&foo)->join() })->join(); say foo()' Artistic AUTHORS AUTHORS AUTHORS

AUTHORS autodoc.pl AUTHORS

So\, it works\, but no clue if this is the proper way of doing it. Talking about proper ways and threads\, I'm trying to write some tests for this\, but I've hit a bit of a conundrum. Test::More requires threads to be pre-loaded to work properly with threads -- but at the same time\, I want to skip all the tests if threads aren't available. I believe the code below is the correct way of doing that\, but it means that most code out there\, including bits in the core\, is doing it wrong. Does that merit patching those files?

use Config; use if $Config{useithreads}\, 'threads'; use Test::More;

BEGIN { if (! $Config{'useithreads'}) { plan skip_all => "Perl not compiled with 'useithreads'"; } }

[*] At least\, that's the impression that I got when I was looking some weeks back\, and what I ended up using in Params::Lazy.

p5pRT commented 10 years ago

From @cpansprout

On Fri Sep 20 18:05:44 2013\, Hugmeir wrote:

On Fri\, Sep 20\, 2013 at 9:16 PM\, Father Chrysostomos via RT \< perlbug-followup@perl.org> wrote:

On Fri Sep 20 16:31:00 2013\, Hugmeir wrote:

Yes. The problem for both is that x_GLOB_ENTRIES should be thread-local\, but is unintentionally being shared between threads.

This patch solves the issue:

diff --git a/ext/File-Glob/Glob.xs b/ext/File-Glob/Glob.xs index b3705b3..181dbbc 100644 --- a/ext/File-Glob/Glob.xs +++ b/ext/File-Glob/Glob.xs @@ -396\,6 +396\,13 @@ PPCODE: iterate(aTHX_ doglob_iter_wrapper); SPAGAIN;

+void +CLONE(...) +CODE: + PERL_UNUSED_ARG(items); + MY_CXT_CLONE; + MY_CXT.x_GLOB_ENTRIES = NULL; + BOOT: { #ifndef PERL_EXTERNAL_GLOB

But it means that this:

./perl -Ilib -Mthreads -E 'sub foo {scalar glob("*")} foo(); say threads->create(\&foo)->join() for 1..3'

Will return "Artistic" four times\, rather than "Artristic" followed by the second file thrice. If the latter is the desired behavior\, then we need to hv_dup(x_GLOB_ENTRIES) in CLONE() as well.

Yes\, the latter. That would be backward-compatible with when File::Glob still worked.

I'm afraid I don't know how to implement this properly! Unlike dup magic\, CLONE() doesn't get a CLONE_PARAMS argument\, so calling sv_dup() and friends requires manual intervention\, and I don't believe that the correct procedure is documented anywhere. The only place in the core that does an sv_dup() during CLONE() is threads.xs\, and that's likely not a good role model for more normal code;

Ah\, but File::Glob is *not* normal code!

meanwhile\, the paradigm in the few CPAN modules that do this[*] is "grab the interpreter during BOOT\, use that to construct a fake CLONE_PARAMS\, and then replace the interpreter with aTHX after MY_CXT_CLONE" Which\, on top of the previous patch\, becomes this:

It has often bothered me that a CLONE method in pure Perl can’t really access both interpreters. I have known about CLONE for a long time\, but had never actually seen it used (or bothered looking).

If what you have below works\, and works reliably (it looks fine to me)\, I say go for it.

Is it possible to write this in a way that is binary-compatible (for 5.16 and 5.18)? I believe the fact that the struct below is not public makes it safe to change it like that.

diff --git a/ext/File-Glob/Glob.xs b/ext/File-Glob/Glob.xs index 181dbbc..118d88e 100644 --- a/ext/File-Glob/Glob.xs +++ b/ext/File-Glob/Glob.xs @@ -9\,6 +9\,9 @@ #define MY_CXT_KEY "File::Glob::_guts" XS_VERSION

typedef struct { +#ifdef USE_ITHREADS + tTHX interp; +#endif int x_GLOB_ERROR; HV * x_GLOB_ENTRIES; Perl_ophook_t x_GLOB_OLD_OPHOOK; @@ -396\,12 +399\,32 @@ PPCODE: iterate(aTHX_ doglob_iter_wrapper); SPAGAIN;

+#ifdef USE_ITHREADS + void CLONE(...) +INIT: + HV *glob_entries_clone = NULL; CODE: PERL_UNUSED_ARG(items); - MY_CXT_CLONE; - MY_CXT.x_GLOB_ENTRIES = NULL; + { + dMY_CXT; + if ( MY_CXT.x_GLOB_ENTRIES ) { + CLONE_PARAMS param; + param.stashes = NULL; + param.flags = 0; + param.proto_perl = MY_CXT.interp; + + glob_entries_clone = MUTABLE_HV(sv_dup_inc((SV*)MY_CXT.x_GLOB_ENTRIES\, &param)); + } + } + { + MY_CXT_CLONE; + MY_CXT.x_GLOB_ENTRIES = glob_entries_clone; + MY_CXT.interp = aTHX; + } + +#endif

BOOT: { @@ -418\,6 +441\,9 @@ BOOT: dMY_CXT; MY_CXT.x_GLOB_ENTRIES = NULL; MY_CXT.x_GLOB_OLD_OPHOOK = PL_opfreehook; +#ifdef USE_ITHREADS + MY_CXT.interp = aTHX; +#endif PL_opfreehook = glob_ophook; } }

And with that: ./perl -Ilib -Mthreads -E 'sub foo {scalar glob("*")} say foo(); say "\t"\, threads->create(\&foo)->join() for 1..3; say ""; say "\t\t"\, threads->create(sub { say "\t"\, foo(); threads->create(\&foo)->join() })->join(); say foo()' Artistic AUTHORS AUTHORS AUTHORS
AUTHORS
    autodoc\.pl
AUTHORS

So\, it works\, but no clue if this is the proper way of doing it. Talking about proper ways and threads\, I'm trying to write some tests for this\, but I've hit a bit of a conundrum. Test::More requires threads to be pre-loaded to work properly with threads -- but at the same time\, I want to skip all the tests if threads aren't available. I believe the code below is the correct way of doing that\, but it means that most code out there\, including bits in the core\, is doing it wrong. Does that merit patching those files?

Probably.

use Config; use if $Config{useithreads}\, 'threads'; use Test::More;

BEGIN { if (! $Config{'useithreads'}) { plan skip_all => "Perl not compiled with 'useithreads'"; } }

[*] At least\, that's the impression that I got when I was looking some weeks back\, and what I ended up using in Params::Lazy.

--

Father Chrysostomos

p5pRT commented 10 years ago

From @cpansprout

On Fri Sep 20 20:19:01 2013\, sprout wrote:

Is it possible to write this in a way that is binary-compatible (for 5.16 and 5.18)? I believe the fact that the struct below is not public makes it safe to change it like that.

That gives the wrong impression. Let’s try again:

I believe (and hope) the proposed changes are binary-compatible. If it turns out I am wrong\, please make them binary-compatible. Thank you.

--

Father Chrysostomos

p5pRT commented 10 years ago

From @Hugmeir

On Sat\, Sep 21\, 2013 at 12:19 AM\, Father Chrysostomos via RT \< perlbug-followup@perl.org> wrote:

On Fri Sep 20 18:05:44 2013\, Hugmeir wrote:

On Fri\, Sep 20\, 2013 at 9:16 PM\, Father Chrysostomos via RT \< perlbug-followup@perl.org> wrote:

On Fri Sep 20 16:31:00 2013\, Hugmeir wrote:

Yes. The problem for both is that x_GLOB_ENTRIES should be thread-local\, but is unintentionally being shared between threads.

This patch solves the issue:

diff --git a/ext/File-Glob/Glob.xs b/ext/File-Glob/Glob.xs index b3705b3..181dbbc 100644 --- a/ext/File-Glob/Glob.xs +++ b/ext/File-Glob/Glob.xs @@ -396\,6 +396\,13 @@ PPCODE: iterate(aTHX_ doglob_iter_wrapper); SPAGAIN;

+void +CLONE(...) +CODE: + PERL_UNUSED_ARG(items); + MY_CXT_CLONE; + MY_CXT.x_GLOB_ENTRIES = NULL; + BOOT: { #ifndef PERL_EXTERNAL_GLOB

But it means that this:

./perl -Ilib -Mthreads -E 'sub foo {scalar glob("*")} foo(); say threads->create(\&foo)->join() for 1..3'

Will return "Artistic" four times\, rather than "Artristic" followed by the second file thrice. If the latter is the desired behavior\, then we need to hv_dup(x_GLOB_ENTRIES) in CLONE() as well.

Yes\, the latter. That would be backward-compatible with when File::Glob still worked.

I'm afraid I don't know how to implement this properly! Unlike dup magic\, CLONE() doesn't get a CLONE_PARAMS argument\, so calling sv_dup() and friends requires manual intervention\, and I don't believe that the correct procedure is documented anywhere. The only place in the core that does an sv_dup() during CLONE() is threads.xs\, and that's likely not a good role model for more normal code;

Ah\, but File::Glob is *not* normal code!

Heh\, fair enough.

meanwhile\, the paradigm in the few CPAN modules that do this[*] is "grab the interpreter during BOOT\, use that to construct a fake CLONE_PARAMS\, and then replace the interpreter with aTHX after MY_CXT_CLONE" Which\, on top of the previous patch\, becomes this:

It has often bothered me that a CLONE method in pure Perl can’t really access both interpreters. I have known about CLONE for a long time\, but had never actually seen it used (or bothered looking).

If what you have below works\, and works reliably (it looks fine to me)\, I say go for it.

I wasn't sure if the new tests would work on VMS -- most of the other glob test files have special cases for it -- so I've pushed this as smoke-me/hugmeir/dup_glob_state

Is it possible to write this in a way that is binary-compatible (for 5.16 and 5.18)? I believe the fact that the struct below is not public makes it safe to change it like that.

This I do not know. (I can't find a definition of binary (in)?compatibility perlpolicy or other pods)

p5pRT commented 10 years ago

From @cpansprout

On Fri Sep 20 23:45:43 2013\, Hugmeir wrote:

On Sat\, Sep 21\, 2013 at 12:19 AM\, Father Chrysostomos via RT \< perlbug-followup@perl.org> wrote:

On Fri Sep 20 18:05:44 2013\, Hugmeir wrote:

meanwhile\, the paradigm in the few CPAN modules that do this[*] is "grab the interpreter during BOOT\, use that to construct a fake CLONE_PARAMS\, and then replace the interpreter with aTHX after MY_CXT_CLONE" Which\, on top of the previous patch\, becomes this:

It has often bothered me that a CLONE method in pure Perl can’t really access both interpreters. I have known about CLONE for a long time\, but had never actually seen it used (or bothered looking).

If what you have below works\, and works reliably (it looks fine to me)\, I say go for it.

I wasn't sure if the new tests would work on VMS -- most of the other glob test files have special cases for it -- so I've pushed this as smoke-me/hugmeir/dup_glob_state

Your test assumes that the output will be sorted\, but does not even use File::Glob on VMS. I don’t know whether the default glob on VMS produces sorted output\, and whether you need to sort it. Alternatively\, just ‘use File::Glob "glob"’ explicitly\, and bypass the globhook mechanism (which VMS ignores) altogether.

Concerning your earlier Test::More comment and my response\, I don’t think we actually need to load threads before Test::More if all the tests will be coming from one thread.

Is it possible to write this in a way that is binary-compatible (for 5.16 and 5.18)? I believe the fact that the struct below is not public makes it safe to change it like that.

This I do not know. (I can't find a definition of binary (in)?compatibility perlpolicy or other pods)

As long as XS extensions compiled with 5.18.[10] continue to work with 5.18.2\, we preserve binary compatibility. With File::Glob\, I don’t think we actually have anything to worry about. I just brought it up because I want to backport the change.

BTW\, we should probably add a new API to perl for registering clone handlers.

--

Father Chrysostomos

p5pRT commented 10 years ago

From @Hugmeir

On Sat\, Sep 21\, 2013 at 10:56 AM\, Father Chrysostomos via RT \< perlbug-followup@perl.org> wrote:

On Fri Sep 20 23:45:43 2013\, Hugmeir wrote:

On Sat\, Sep 21\, 2013 at 12:19 AM\, Father Chrysostomos via RT \< perlbug-followup@perl.org> wrote:

On Fri Sep 20 18:05:44 2013\, Hugmeir wrote:

meanwhile\, the paradigm in the few CPAN modules that do this[*] is "grab the interpreter during BOOT\, use that to construct a fake CLONE_PARAMS\, and then replace the interpreter with aTHX after MY_CXT_CLONE" Which\, on top of the previous patch\, becomes this:

It has often bothered me that a CLONE method in pure Perl can’t really access both interpreters. I have known about CLONE for a long time\, but had never actually seen it used (or bothered looking).

If what you have below works\, and works reliably (it looks fine to me)\, I say go for it.

I wasn't sure if the new tests would work on VMS -- most of the other glob test files have special cases for it -- so I've pushed this as smoke-me/hugmeir/dup_glob_state

Your test assumes that the output will be sorted\, but does not even use File::Glob on VMS. I don’t know whether the default glob on VMS produces sorted output\, and whether you need to sort it. Alternatively\, just ‘use File::Glob "glob"’ explicitly\, and bypass the globhook mechanism (which VMS ignores) altogether.

Ah\, I see\, thanks. From the docs\, I had assumed that it always gave sorted output by default -- perhaps that needs updating to have a clause for VMS if that's not the case. I've modified the branch to have a 'use File::Glob "glob"'.

Concerning your earlier Test::More comment and my response\, I don’t think we actually need to load threads before Test::More if all the tests will be coming from one thread.

While that's the case right now\, the problem is that it's a potential future gotcha for anyone adding more tests.

Is it possible to write this in a way that is binary-compatible (for 5.16 and 5.18)? I believe the fact that the struct below is not public makes it safe to change it like that.

This I do not know. (I can't find a definition of binary (in)?compatibility perlpolicy or other pods)

As long as XS extensions compiled with 5.18.[10] continue to work with 5.18.2\, we preserve binary compatibility. With File::Glob\, I don’t think we actually have anything to worry about. I just brought it up because I want to backport the change.

BTW\, we should probably add a new API to perl for registering clone handlers.

+50

p5pRT commented 10 years ago

From @cpansprout

On Sat Sep 21 07:40:33 2013\, Hugmeir wrote:

On Sat\, Sep 21\, 2013 at 10:56 AM\, Father Chrysostomos via RT \< perlbug-followup@perl.org> wrote:

Your test assumes that the output will be sorted\, but does not even use File::Glob on VMS. I don’t know whether the default glob on VMS produces sorted output\, and whether you need to sort it. Alternatively\, just ‘use File::Glob "glob"’ explicitly\, and bypass the globhook mechanism (which VMS ignores) altogether.

Ah\, I see\, thanks. From the docs\, I had assumed that it always gave sorted output by default -- perhaps that needs updating to have a clause for VMS if that's not the case. I've modified the branch to have a 'use File::Glob "glob"'.

I think perlfunc/glob needs to state that glob is not implemented via File::Glob on VMS. I don’t know how much of the text there about whitespace is applicable to VMS’s default glob\, so I am not really qualified to write it.

--

Father Chrysostomos

p5pRT commented 10 years ago

From @craigberry

On Sat\, Sep 21\, 2013 at 12:51 PM\, Father Chrysostomos via RT \perlbug\-followup@perl\.org wrote:

On Sat Sep 21 07:40:33 2013\, Hugmeir wrote:

I have done a smoke run of smoke-me/hugmeir/dup_glob_state\, results available at:

\<http://www.nntp.perl.org/group/perl.daily-build.reports/2013/09/msg151124.html>

I mention that here because unless you have the SHA1s memorized\, there is no way from looking at it that you would know what branch was smoked. All the failures there are also happening in blead\, i.e.\, no new test failures.

On Sat\, Sep 21\, 2013 at 10:56 AM\, Father Chrysostomos via RT \< perlbug-followup@perl.org> wrote:

Your test assumes that the output will be sorted\, but does not even use File::Glob on VMS. I don’t know whether the default glob on VMS produces sorted output\, and whether you need to sort it.

It uses the results of LIB$FIND_FILE\, documented at:

\<http://h71000.www7.hp.com/doc/82final/5932/5932pro_017.html#fin_f>

but I see no mention of sorting there. As far as I've ever seen\, it's the order the files appear in the directory file\, which on any system I've ever worked on means asciibetically. It's possible different locales or local character sets could change the physical order of the directory file\, but I don't know that. If it's important I could try to find out.

Alternatively\, just ‘use File::Glob "glob"’ explicitly\, and bypass the globhook mechanism (which VMS ignores) altogether.

Ah\, I see\, thanks. From the docs\, I had assumed that it always gave sorted output by default -- perhaps that needs updating to have a clause for VMS if that's not the case. I've modified the branch to have a 'use File::Glob "glob"'.

I think perlfunc/glob needs to state that glob is not implemented via File::Glob on VMS.

Does perlfunc/glob really need to say how it's implemented? To me that implies we're not quite sure how it actually works or maybe never decided how it should work.

I don’t know how much of the text there about whitespace is applicable to VMS’s default glob\, so I am not really qualified to write it.

On VMS\, glob does not split its arguments on whitespace:

$ perl -e "print glob('*.c');" av.cdeb.cdoio.cdoop.cdquote_static.cdump.cgenerate_uudmap.cglobals.cgv.chv.cinline_invlist.ckeywords.clocale.cmadly.cmalloc.cmathoms.cmg.cmg_names.c miniperlmain.cmro.cnumeric.cop.coverload.cpacksizetables.cpad.cperl.cperlapi.cperlio.cperlmain.cPERLMINI.Cperly.cpp.cpp_ctl.cpp_hot.cpp_pack.cpp_sor t.cpp_sys.creentr.cregcomp.cregexec.crun.cscope.csv.ctaint.ctime64.ctoke.cuniversal.cutf8.cutil.cvms.c $ perl -e "print glob('*.c *.h');" $

I don't know when the split on whitespace feature was introduced but it was probably an oversight or lack of tuits that kept it from getting added to the VMS implementation.

Whitespace in the pattern is assumed to be part of the filename\, and that does work:

$ create file^ with_space.txt ^Z $ perl -e "print glob('file with*');" file^_with_space.txt

where "^_" is the canonical form of a caret-escaped space character.

p5pRT commented 10 years ago

From @Hugmeir

On Sat\, Sep 21\, 2013 at 10:56 AM\, Father Chrysostomos via RT \< perlbug-followup@perl.org> wrote:

On Fri Sep 20 23:45:43 2013\, Hugmeir wrote:

On Sat\, Sep 21\, 2013 at 12:19 AM\, Father Chrysostomos via RT \< perlbug-followup@perl.org> wrote:

On Fri Sep 20 18:05:44 2013\, Hugmeir wrote:

meanwhile\, the paradigm in the few CPAN modules that do this[*] is "grab the interpreter during BOOT\, use that to construct a fake CLONE_PARAMS\, and then replace the interpreter with aTHX after MY_CXT_CLONE" Which\, on top of the previous patch\, becomes this:

It has often bothered me that a CLONE method in pure Perl can’t really access both interpreters. I have known about CLONE for a long time\, but had never actually seen it used (or bothered looking).

If what you have below works\, and works reliably (it looks fine to me)\, I say go for it.

I wasn't sure if the new tests would work on VMS -- most of the other glob test files have special cases for it -- so I've pushed this as smoke-me/hugmeir/dup_glob_state

Your test assumes that the output will be sorted\, but does not even use File::Glob on VMS. I don’t know whether the default glob on VMS produces sorted output\, and whether you need to sort it. Alternatively\, just ‘use File::Glob "glob"’ explicitly\, and bypass the globhook mechanism (which VMS ignores) altogether.

Actually. I've hit a bit of a snag here. How come bsd_glob() doesn't keep state?

use 5.010; use File::Glob qw(bsd_glob csh_glob);

say "Default glob:"; say scalar glob("*") for 1..3;

say "\nbsd_glob:"; say scalar bsd_glob("*") for 1..3;

say "\ncsh_glob:"; say scalar csh_glob("*") for 1..3;

__END__ Default glob: base benchmark bigmem

bsd_glob: x2p x2p x2p

csh_glob: base benchmark bigmem

Also\, by doing a 'use File::Glob qw(:bsd_glob)'\, CORE::glob gets replaced with bsd_glob_override()\, which does keep state. So bsd_glob() sometimes keeps state?

I can work around this by using csh_glob in those tests\, which both keeps state and returns the results sorted alphabetically\, but I don't quite grok how this is all supposed to work.

Concerning your earlier Test::More comment and my response\, I don’t think we actually need to load threads before Test::More if all the tests will be coming from one thread.

Is it possible to write this in a way that is binary-compatible (for 5.16 and 5.18)? I believe the fact that the struct below is not public makes it safe to change it like that.

This I do not know. (I can't find a definition of binary (in)?compatibility perlpolicy or other pods)

As long as XS extensions compiled with 5.18.[10] continue to work with 5.18.2\, we preserve binary compatibility. With File::Glob\, I don’t think we actually have anything to worry about. I just brought it up because I want to backport the change.

BTW\, we should probably add a new API to perl for registering clone handlers.

--

Father Chrysostomos

--- via perlbug: queue: perl5 status: open https://rt-archive.perl.org/perl5/Ticket/Display.html?id=119897

p5pRT commented 10 years ago

From kaffeetisch@gmx.de

On 21.09.2013 15:56\, Father Chrysostomos via RT wrote:

BTW\, we should probably add a new API to perl for registering clone handlers.

Note that there is a kind of API for SV-specific cloning: the MGf_DUP magic flag.

p5pRT commented 10 years ago

From @cpansprout

On Sat Sep 21 19:48:07 2013\, craig.a.berry@gmail.com wrote:

On Sat\, Sep 21\, 2013 at 12:51 PM\, Father Chrysostomos via RT \perlbug\-followup@perl\.org wrote:

On Sat Sep 21 07:40:33 2013\, Hugmeir wrote:

I have done a smoke run of smoke-me/hugmeir/dup_glob_state\, results available at:

\<http://www.nntp.perl.org/group/perl.daily- build.reports/2013/09/msg151124.html>

I mention that here because unless you have the SHA1s memorized\, there is no way from looking at it that you would know what branch was smoked. All the failures there are also happening in blead\, i.e.\, no new test failures.

On Sat\, Sep 21\, 2013 at 10:56 AM\, Father Chrysostomos via RT \< perlbug-followup@perl.org> wrote:

Your test assumes that the output will be sorted\, but does not even use File::Glob on VMS. I don’t know whether the default glob on VMS produces sorted output\, and whether you need to sort it.

It uses the results of LIB$FIND_FILE\, documented at:

\<http://h71000.www7.hp.com/doc/82final/5932/5932pro_017.html#fin_f>

but I see no mention of sorting there. As far as I've ever seen\, it's the order the files appear in the directory file\, which on any system I've ever worked on means asciibetically. It's possible different locales or local character sets could change the physical order of the directory file\, but I don't know that. If it's important I could try to find out.

I don’t think it’s important. My main point was that the test intended to test File::Glob itself wasn’t testing it at all on VMS.

Alternatively\, just ‘use File::Glob "glob"’ explicitly\, and bypass the globhook mechanism (which VMS ignores) altogether.

Ah\, I see\, thanks. From the docs\, I had assumed that it always gave sorted output by default -- perhaps that needs updating to have a clause for VMS if that's not the case. I've modified the branch to have a 'use File::Glob "glob"'.

I think perlfunc/glob needs to state that glob is not implemented via File::Glob on VMS.

Does perlfunc/glob really need to say how it's implemented? To me that implies we're not quite sure how it actually works or maybe never decided how it should work.

Currently we state that it is implemented via File::Glob and one can see File::Glob for more information. That itself is useful\, because\, for non-VMS users\, ‘use File::Glob ":nocase"’ has a process-wide effect on the glob operator.

I don’t know how much of the text there about whitespace is applicable to VMS’s default glob\, so I am not really qualified to write it.

On VMS\, glob does not split its arguments on whitespace:

$ perl -e "print glob('*.c');"

av.cdeb.cdoio.cdoop.cdquote_static.cdump.cgenerate_uudmap.cglobals.cgv.chv.cinline_invlist.ckeywords.clocale.cmadly.cmalloc.cmathoms.cmg.cmg_names.c

miniperlmain.cmro.cnumeric.cop.coverload.cpacksizetables.cpad.cperl.cperlapi.cperlio.cperlmain.cPERLMINI.Cperly.cpp.cpp_ctl.cpp_hot.cpp_pack.cpp_sor

t.cpp_sys.creentr.cregcomp.cregexec.crun.cscope.csv.ctaint.ctime64.ctoke.cuniversal.cutf8.cutil.cvms.c $ perl -e "print glob('*.c *.h');" $

I don't know when the split on whitespace feature was introduced

perl 3\, which introduced glob.

but it was probably an oversight or lack of tuits that kept it from getting added to the VMS implementation.

--

Father Chrysostomos

p5pRT commented 10 years ago

From @cpansprout

On Sun Sep 22 02:28:24 2013\, Hugmeir wrote:

Actually. I've hit a bit of a snag here. How come bsd_glob() doesn't keep state?

use 5.010; use File::Glob qw(bsd_glob csh_glob);

say "Default glob:"; say scalar glob("*") for 1..3;

say "\nbsd_glob:"; say scalar bsd_glob("*") for 1..3;

say "\ncsh_glob:"; say scalar csh_glob("*") for 1..3;

__END__ Default glob: base benchmark bigmem

bsd_glob: x2p x2p x2p

csh_glob: base benchmark bigmem

Also\, by doing a 'use File::Glob qw(:bsd_glob)'\, CORE::glob gets replaced with bsd_glob_override()\, which does keep state. So bsd_glob() sometimes keeps state?

I can work around this by using csh_glob in those tests\, which both keeps state and returns the results sorted alphabetically\, but I don't quite grok how this is all supposed to work.

It’s an historical mess. It used to be that keeping state and splitting the pattern went hand in hand. There was no way to bypass skipping the pattern *and* have it keep state.

I suggested adding the :bsd_glob tag (since bsd_glob was known not to split)\, which got support on the list\, so I added it. It makes \<...> bypass splitting.

But File::Glob’s interface was a confusing mess to begin with\, and we can’t clean it up to begin with.

--

Father Chrysostomos

p5pRT commented 10 years ago

From @epa

Craig A. Berry \<craig.a.berry \ gmail.com> writes:

On VMS\, glob does not split its arguments on whitespace:

I don't know when the split on whitespace feature was introduced but it was probably an oversight or lack of tuits that kept it from getting added to the VMS implementation.

AFAIK\, glob splitting on whitespace was an accident of its original implementation (which worked by running an external tcsh) and was preserved for compatibility when glob became implemented by Perl code.

Personally I'm not a fan of this behaviour - it tends to cause bugs when file or directory names contain spaces\, as is nowadays common on Windows and Mac systems. The fact that split-at-space has never happened on VMS might be another argument towards deprecating it.

-- Ed Avis \eda@waniasset\.com

p5pRT commented 10 years ago

From @Hugmeir

On Sat\, Sep 21\, 2013 at 11:47 PM\, Craig A. Berry \craig\.a\.berry@gmail\.comwrote:

On Sat\, Sep 21\, 2013 at 12:51 PM\, Father Chrysostomos via RT \perlbug\-followup@perl\.org wrote:

On Sat Sep 21 07:40:33 2013\, Hugmeir wrote:

I have done a smoke run of smoke-me/hugmeir/dup_glob_state\, results available at:

\< http://www.nntp.perl.org/group/perl.daily-build.reports/2013/09/msg151124.html

I mention that here because unless you have the SHA1s memorized\, there is no way from looking at it that you would know what branch was smoked. All the failures there are also happening in blead\, i.e.\, no new test failures.

Thanks! I would've entirely missed that. Looks like under threads\, the new tests is failing like Father C predicted... Could I ask you to fetch the branch again and try that test?

On Sat\, Sep 21\, 2013 at 10:56 AM\, Father Chrysostomos via RT \< perlbug-followup@perl.org> wrote:

Your test assumes that the output will be sorted\, but does not even use File::Glob on VMS. I don’t know whether the default glob on VMS produces sorted output\, and whether you need to sort it.

It uses the results of LIB$FIND_FILE\, documented at:

\<http://h71000.www7.hp.com/doc/82final/5932/5932pro_017.html#fin_f>

but I see no mention of sorting there. As far as I've ever seen\, it's the order the files appear in the directory file\, which on any system I've ever worked on means asciibetically. It's possible different locales or local character sets could change the physical order of the directory file\, but I don't know that. If it's important I could try to find out.

Alternatively\, just ‘use File::Glob "glob"’ explicitly\, and bypass the globhook mechanism (which VMS ignores) altogether.

Ah\, I see\, thanks. From the docs\, I had assumed that it always gave sorted output by default -- perhaps that needs updating to have a clause for VMS if that's not the case. I've modified the branch to have a 'use File::Glob "glob"'.

I think perlfunc/glob needs to state that glob is not implemented via File::Glob on VMS.

Does perlfunc/glob really need to say how it's implemented?

I think that it would belong in perlport\, but perlport for glob points to File::Glob\, so it would all go there.

To me that implies we're not quite sure how it actually works or maybe never decided how it should work.

I don’t know how much of the text there about whitespace is applicable to VMS’s default glob\, so I am not really qualified to write it.

On VMS\, glob does not split its arguments on whitespace:

$ perl -e "print glob('*.c');"

av.cdeb.cdoio.cdoop.cdquote_static.cdump.cgenerate_uudmap.cglobals.cgv.chv.cinline_invlist.ckeywords.clocale.cmadly.cmalloc.cmathoms.cmg.cmg_names.c

miniperlmain.cmro.cnumeric.cop.coverload.cpacksizetables.cpad.cperl.cperlapi.cperlio.cperlmain.cPERLMINI.Cperly.cpp.cpp_ctl.cpp_hot.cpp_pack.cpp_sor

t.cpp_sys.creentr.cregcomp.cregexec.crun.cscope.csv.ctaint.ctime64.ctoke.cuniversal.cutf8.cutil.cvms.c $ perl -e "print glob('*.c *.h');" $

I don't know when the split on whitespace feature was introduced but it was probably an oversight or lack of tuits that kept it from getting added to the VMS implementation.

Whitespace in the pattern is assumed to be part of the filename\, and that does work:

$ create file^ with_space.txt ^Z $ perl -e "print glob('file with*');" file^_with_space.txt

where "^_" is the canonical form of a caret-escaped space character.

Honestly that seems more useful than splitting on whitespace. Windows uses (and users use) spaces in filenames all the time\, and whenever I do any sort of collab with a non-programmer those inevitably creep in.

p5pRT commented 10 years ago

From @ikegami

On Tue\, Sep 24\, 2013 at 9:32 AM\, Brian Fraser \fraserbn@gmail\.com wrote:

Honestly that seems more useful than splitting on whitespace. Windows uses (and users use) spaces in filenames all the time\, and whenever I do any sort of collab with a non-programmer those inevitably creep in.

Wouldn't be a big issue if you could escape the space\, but you can't. Gotta use bsd_glob instead.

perl -E"say for glob 'c:\\program files\\*'" c:\program

perl -E"say for glob 'c:\\program\ files\\*'"

perl -MFile::Glob=bsd_glob -E"say for bsd_glob 'c:\\program files\\*'" | find "Common" c:\program files\Common Files

p5pRT commented 10 years ago

From @ikegami

On Tue\, Sep 24\, 2013 at 1:36 PM\, Eric Brine \ikegami@adaelis\.com wrote:

On Tue\, Sep 24\, 2013 at 9:32 AM\, Brian Fraser \fraserbn@gmail\.com wrote:

Honestly that seems more useful than splitting on whitespace. Windows uses (and users use) spaces in filenames all the time\, and whenever I do any sort of collab with a non-programmer those inevitably creep in.

Wouldn't be a big issue if you could escape the space\, but you can't. Gotta use bsd_glob instead.

perl -E"say for glob 'c:\\program files\\*'" c:\program

perl -E"say for glob 'c:\\program\ files\\*'"

perl -MFile::Glob=bsd_glob -E"say for bsd_glob 'c:\\program files\\*'" | find "Common" c:\program files\Common Files

Let me rephrase that. You can't *on Windows*. There's no problem escaping spaces on unix systems.

$ perl -E'say for glob "a\ b/*"' a

p5pRT commented 10 years ago

From @mauke

On 24.09.2013 14:27\, Ed Avis wrote:

AFAIK\, glob splitting on whitespace was an accident of its original implementation (which worked by running an external tcsh) and was preserved for compatibility when glob became implemented by Perl code.

Personally I'm not a fan of this behaviour - it tends to cause bugs when file or directory names contain spaces\, as is nowadays common on Windows and Mac systems. The fact that split-at-space has never happened on VMS might be another argument towards deprecating it.

Yes\, please! You can easily get the old behavior of glob "foo bar" with glob("foo")\, glob("bar").

-- Lukas Mai \plokinom@gmail\.com

p5pRT commented 10 years ago

From @ikegami

On Tue\, Sep 24\, 2013 at 3:02 PM\, Lukas Mai \plokinom@gmail\.com wrote:

Yes\, please! You can easily get the old behavior of glob "foo bar" with glob("foo")\, glob("bar").

Why break backwards compatibility just to save yourself from escaping spaces when you must already escape other characters such as "{"?

p5pRT commented 10 years ago

From @tux

On Tue\, 24 Sep 2013 21:02:45 +0200\, Lukas Mai \plokinom@gmail\.com wrote:

On 24.09.2013 14:27\, Ed Avis wrote:

AFAIK\, glob splitting on whitespace was an accident of its original implementation (which worked by running an external tcsh) and was preserved for compatibility when glob became implemented by Perl code.

Personally I'm not a fan of this behaviour - it tends to cause bugs when file or directory names contain spaces\, as is nowadays common on Windows and Mac systems. The fact that split-at-space has never happened on VMS might be another argument towards deprecating it.

Yes\, please! You can easily get the old behavior of glob "foo bar" with glob("foo")\, glob("bar").

No\, please not. That would break

foreach my $filename (\<a b c d* ef* >) {

which would extend to quite a long and ugly line when you have to rewrite those to globs where parens are required.

-- H.Merijn Brand http://tux.nl Perl Monger http://amsterdam.pm.org/ using perl5.00307 .. 5.19 porting perl5 on HP-UX\, AIX\, and openSUSE http://mirrors.develooper.com/hpux/ http://www.test-smoke.org/ http://qa.perl.org http://www.goldmark.org/jeff/stupid-disclaimers/

p5pRT commented 10 years ago

From @craigberry

On Tue\, Sep 24\, 2013 at 8:32 AM\, Brian Fraser \fraserbn@gmail\.com wrote:

On Sat\, Sep 21\, 2013 at 11:47 PM\, Craig A. Berry \craig\.a\.berry@gmail\.com wrote:

On Sat\, Sep 21\, 2013 at 12:51 PM\, Father Chrysostomos via RT \perlbug\-followup@perl\.org wrote:

On Sat Sep 21 07:40:33 2013\, Hugmeir wrote:

I have done a smoke run of smoke-me/hugmeir/dup_glob_state\, results available at:

\<http://www.nntp.perl.org/group/perl.daily-build.reports/2013/09/msg151124.html>

Could I ask you to fetch the branch again and try that test?

Here ha go:

\<http://www.nntp.perl.org/group/perl.daily-build.reports/2013/09/msg151317.html>

The test failure I think you're interested in looks like:

$ perl ../ext/File-Glob/t/threads.t not ok 1 # Failed test at ../ext/File-Glob/t/threads.t line 44. # got: '1_file.' # expected: '1_file' not ok 2 - glob() state is cloned for new threads # Failed test 'glob() state is cloned for new threads' # at ../ext/File-Glob/t/threads.t line 48. # Structures begin differing at: # $got->[0] = '2_file.' # $expected->[0] = '2_file' not ok 3 - ..and for new threads inside threads # Failed test '..and for new threads inside threads' # at ../ext/File-Glob/t/threads.t line 59. # Structures begin differing at: # $got->[0] = '2_file.' # $expected->[0] = '2_file' not ok 4 - state doesn't leak from threads # Failed test 'state doesn't leak from threads' # at ../ext/File-Glob/t/threads.t line 66. # got: '2_file.' # expected: '2_file' 1..4 # Looks like you failed 4 tests of 4. %SYSTEM-F-ABORT\, abort $

And the gotcha is that on VMS\, all files have extensions\, even those that don't :-). In other words\, the trailing dot indicates a zero-length extension. The path of least resistance is to just put an explicit extension on those files:

Inline Patch

```diff --- ext/File-Glob/t/threads.t;-0 2013-09-22 05:26:48 -0500 +++ ext/File-Glob/t/threads.t 2013-09-25 07:08:29 -0500 @@ -28,7 +28,7 @@ use File::Glob qw(csh_glob); my($dir) = tempdir(CLEANUP => 1) or die "Could not create temporary directory"; -my @temp_files = qw(1_file 2_file 3_file); +my @temp_files = qw(1_file.tmp 2_file.tmp 3_file.tmp); for my $file (@temp_files) { open my $fh, ">", File::Spec->catfile($dir, $file) or die "Could not create file $dir/$file: $!"; ```

[end]

which gets the test to pass:

$ perl ../ext/File-Glob/t/threads.t ok 1 ok 2 - glob() state is cloned for new threads ok 3 - ..and for new threads inside threads ok 4 - state doesn't leak from threads 1..4

p5pRT commented 10 years ago

From @Hugmeir

On Wed\, Sep 25\, 2013 at 9:14 AM\, Craig A. Berry \craig\.a\.berry@gmail\.comwrote:

On Tue\, Sep 24\, 2013 at 8:32 AM\, Brian Fraser \fraserbn@gmail\.com wrote:

On Sat\, Sep 21\, 2013 at 11:47 PM\, Craig A. Berry \< craig.a.berry@gmail.com> wrote:

On Sat\, Sep 21\, 2013 at 12:51 PM\, Father Chrysostomos via RT \perlbug\-followup@perl\.org wrote:

On Sat Sep 21 07:40:33 2013\, Hugmeir wrote:

I have done a smoke run of smoke-me/hugmeir/dup_glob_state\, results available at:

\< http://www.nntp.perl.org/group/perl.daily-build.reports/2013/09/msg151124.html

Could I ask you to fetch the branch again and try that test?

Here ha go:

\< http://www.nntp.perl.org/group/perl.daily-build.reports/2013/09/msg151317.html

The test failure I think you're interested in looks like:

$ perl ../ext/File-Glob/t/threads.t not ok 1 # Failed test at ../ext/File-Glob/t/threads.t line 44. # got: '1_file.' # expected: '1_file' not ok 2 - glob() state is cloned for new threads # Failed test 'glob() state is cloned for new threads' # at ../ext/File-Glob/t/threads.t line 48. # Structures begin differing at: # $got->[0] = '2_file.' # $expected->[0] = '2_file' not ok 3 - ..and for new threads inside threads # Failed test '..and for new threads inside threads' # at ../ext/File-Glob/t/threads.t line 59. # Structures begin differing at: # $got->[0] = '2_file.' # $expected->[0] = '2_file' not ok 4 - state doesn't leak from threads # Failed test 'state doesn't leak from threads' # at ../ext/File-Glob/t/threads.t line 66. # got: '2_file.' # expected: '2_file' 1..4 # Looks like you failed 4 tests of 4. %SYSTEM-F-ABORT\, abort $

And the gotcha is that on VMS\, all files have extensions\, even those that don't :-). In other words\, the trailing dot indicates a zero-length extension. The path of least resistance is to just put an explicit extension on those files:

--- ext/File-Glob/t/threads.t;-0 2013-09-22 05:26:48 -0500 +++ ext/File-Glob/t/threads.t 2013-09-25 07:08:29 -0500 @@ -28\,7 +28\,7 @@ use File::Glob qw(csh_glob); my($dir) = tempdir(CLEANUP => 1) or die "Could not create temporary directory";

-my @temp_files = qw(1_file 2_file 3_file); +my @temp_files = qw(1_file.tmp 2_file.tmp 3_file.tmp); for my $file (@temp_files) { open my $fh\, ">"\, File::Spec->catfile($dir\, $file) or die "Could not create file $dir/$file: $!"; [end]

which gets the test to pass:

$ perl ../ext/File-Glob/t/threads.t ok 1 ok 2 - glob() state is cloned for new threads ok 3 - ..and for new threads inside threads ok 4 - state doesn't leak from threads 1..4

Ooh\, thank you! I'll push the branch with those changes later today. (As a side note\, VMS actually sounds like a pretty fun system; I wish that I could just emulate it locally)

p5pRT commented 10 years ago

From @mauke

On 25.09.2013 08:07\, H.Merijn Brand wrote:

On Tue\, 24 Sep 2013 21:02:45 +0200\, Lukas Mai \plokinom@gmail\.com wrote:

Yes\, please! You can easily get the old behavior of glob "foo bar" with glob("foo")\, glob("bar").

No\, please not. That would break

foreach my $filename (\<a b c d* ef* >) {

which would extend to quite a long and ugly line when you have to rewrite those to globs where parens are required.

for my $filename (map glob\, qw\<a b c d* ef*>) {

-- Lukas Mai \plokinom@gmail\.com

p5pRT commented 10 years ago

@Hugmeir - Status changed from 'open' to 'resolved'

p5pRT commented 10 years ago

From @Hugmeir

On Wed\, Sep 25\, 2013 at 9:51 AM\, Brian Fraser \fraserbn@gmail\.com wrote:

On Wed\, Sep 25\, 2013 at 9:14 AM\, Craig A. Berry \craig\.a\.berry@gmail\.comwrote:

On Tue\, Sep 24\, 2013 at 8:32 AM\, Brian Fraser \fraserbn@gmail\.com wrote:

On Sat\, Sep 21\, 2013 at 11:47 PM\, Craig A. Berry \< craig.a.berry@gmail.com> wrote:

On Sat\, Sep 21\, 2013 at 12:51 PM\, Father Chrysostomos via RT \perlbug\-followup@perl\.org wrote:

On Sat Sep 21 07:40:33 2013\, Hugmeir wrote:

I have done a smoke run of smoke-me/hugmeir/dup_glob_state\, results available at:

\< http://www.nntp.perl.org/group/perl.daily-build.reports/2013/09/msg151124.html

Could I ask you to fetch the branch again and try that test?

Here ha go:

\< http://www.nntp.perl.org/group/perl.daily-build.reports/2013/09/msg151317.html

The test failure I think you're interested in looks like:

$ perl ../ext/File-Glob/t/threads.t not ok 1 # Failed test at ../ext/File-Glob/t/threads.t line 44. # got: '1_file.' # expected: '1_file' not ok 2 - glob() state is cloned for new threads # Failed test 'glob() state is cloned for new threads' # at ../ext/File-Glob/t/threads.t line 48. # Structures begin differing at: # $got->[0] = '2_file.' # $expected->[0] = '2_file' not ok 3 - ..and for new threads inside threads # Failed test '..and for new threads inside threads' # at ../ext/File-Glob/t/threads.t line 59. # Structures begin differing at: # $got->[0] = '2_file.' # $expected->[0] = '2_file' not ok 4 - state doesn't leak from threads # Failed test 'state doesn't leak from threads' # at ../ext/File-Glob/t/threads.t line 66. # got: '2_file.' # expected: '2_file' 1..4 # Looks like you failed 4 tests of 4. %SYSTEM-F-ABORT\, abort $

And the gotcha is that on VMS\, all files have extensions\, even those that don't :-). In other words\, the trailing dot indicates a zero-length extension. The path of least resistance is to just put an explicit extension on those files:

--- ext/File-Glob/t/threads.t;-0 2013-09-22 05:26:48 -0500 +++ ext/File-Glob/t/threads.t 2013-09-25 07:08:29 -0500 @@ -28\,7 +28\,7 @@ use File::Glob qw(csh_glob); my($dir) = tempdir(CLEANUP => 1) or die "Could not create temporary directory";

-my @temp_files = qw(1_file 2_file 3_file); +my @temp_files = qw(1_file.tmp 2_file.tmp 3_file.tmp); for my $file (@temp_files) { open my $fh\, ">"\, File::Spec->catfile($dir\, $file) or die "Could not create file $dir/$file: $!"; [end]

which gets the test to pass:

$ perl ../ext/File-Glob/t/threads.t ok 1 ok 2 - glob() state is cloned for new threads ok 3 - ..and for new threads inside threads ok 4 - state doesn't leak from threads 1..4

Ooh\, thank you! I'll push the branch with those changes later today. (As a side note\, VMS actually sounds like a pretty fun system; I wish that I could just emulate it locally)

For some loose definition of "today." Fixed in facf34e on blead.

p5pRT commented 10 years ago

From @nwc10

On Wed\, Sep 25\, 2013 at 09:51:42AM -0300\, Brian Fraser wrote:

Ooh\, thank you! I'll push the branch with those changes later today. (As a side note\, VMS actually sounds like a pretty fun system; I wish that I could just emulate it locally)

In theory you could run it on emulated hardware\, as there is a "hobbyist license" available. But it would be more practical to figure out how to get an account on HP's OpenVMS Open Source Cluster. (Cluster - hmmm\, only the IA64 machine seems to be up these days)

I have an account\, but I'm not sure what the current procedure is to get one\, as I think mine dates back to the enlightened time when HP provided an entire "Test Drive" service\, with most of the hardware and OS combinations they offered available.

Nicholas Clark

p5pRT commented 10 years ago

From @craigberry

On Fri\, Sep 27\, 2013 at 6:22 AM\, Nicholas Clark \nick@ccl4\.org wrote:

On Wed\, Sep 25\, 2013 at 09:51:42AM -0300\, Brian Fraser wrote:

Ooh\, thank you! I'll push the branch with those changes later today. (As a side note\, VMS actually sounds like a pretty fun system; I wish that I could just emulate it locally)

In theory you could run it on emulated hardware\, as there is a "hobbyist license" available.

There are various Alpha emulators around and I think they all have a free option\, usually somewhat limited in memory (can mktables do its think in 128MB?). If you have recent enough HP-UX kit\, you can run instances of OpenVMS I64 under HPVM\, but then if you have that kind of kit\, you can run it without emulation.

But it would be more practical to figure out how to get an account on HP's OpenVMS Open Source Cluster. (Cluster - hmmm\, only the IA64 machine seems to be up these days)

And I believe it's a pretty underpowered ten-year-old Integrity server\, but yes\, someone else acquired it and set it up\, so it's the quickest way to get started.

I have an account\, but I'm not sure what the current procedure is to get one\, as I think mine dates back to the enlightened time when HP provided an entire "Test Drive" service\, with most of the hardware and OS combinations they offered available.

I think all you have to do is drop a note to OpenSource DOT OpenVMS AT hp.com\, tell them what you're working on\, and request an account.

p5pRT commented 10 years ago

From @cpansprout

On Fri Sep 27 03:59:08 2013\, Hugmeir wrote:

For some loose definition of "today." Fixed in facf34e on blead.

Is there any chance of backporting that to 5.18 and 5.16?

--

Father Chrysostomos

p5pRT commented 10 years ago

From @Hugmeir

On Sat\, Sep 28\, 2013 at 10:31 AM\, Father Chrysostomos via RT \< perlbug-followup@perl.org> wrote:

On Fri Sep 27 03:59:08 2013\, Hugmeir wrote:

For some loose definition of "today." Fixed in facf34e on blead.

Is there any chance of backporting that to 5.18 and 5.16?

+1 to the cherrypick. This is a regression from 5.14; the commit applies cleanly to 5.18\, and needs only a trivial change in 5.16.

p5pRT commented 10 years ago

From @rjbs

* Brian Fraser \fraserbn@gmail\.com [2013-09-28T09:45:14]

On Sat\, Sep 28\, 2013 at 10:31 AM\, Father Chrysostomos via RT \< perlbug-followup@perl.org> wrote:

On Fri Sep 27 03:59:08 2013\, Hugmeir wrote:

For some loose definition of "today." Fixed in facf34e on blead.

Is there any chance of backporting that to 5.18 and 5.16?

+1 to the cherrypick. This is a regression from 5.14; the commit applies cleanly to 5.18\, and needs only a trivial change in 5.16.

Please apply them the to maint branches.

I'll be posting about 5.18.2 soon.

-- rjbs

p5pRT commented 10 years ago

From @nwc10

On Fri\, Sep 27\, 2013 at 09:01:51AM -0500\, Craig A. Berry wrote:

On Fri\, Sep 27\, 2013 at 6:22 AM\, Nicholas Clark \nick@ccl4\.org wrote:

On Wed\, Sep 25\, 2013 at 09:51:42AM -0300\, Brian Fraser wrote:

Ooh\, thank you! I'll push the branch with those changes later today. (As a side note\, VMS actually sounds like a pretty fun system; I wish that I could just emulate it locally)

In theory you could run it on emulated hardware\, as there is a "hobbyist license" available.

There are various Alpha emulators around and I think they all have a free option\, usually somewhat limited in memory (can mktables do its think in 128MB?). If you have recent enough HP-UX kit\, you can run

I've run it on a 32 bit system that only has 16M of real RAM and 96M of swap. 128M of RAM would be a luxury :-)

(No-one said that it was *fast*. Nor was it quiet. But it completed)

You could probably build Perl 5 natively on most smartphones released in the past 3 years\, if they had an OS with a toolchain. Hardware is getting bloated faster than we are.

Nicholas Clark

p5pRT commented 10 years ago

From @craigberry

On Fri\, Sep 27\, 2013 at 5:58 AM\, Brian Fraser \fraserbn@gmail\.com wrote:

On Wed\, Sep 25\, 2013 at 9:51 AM\, Brian Fraser \fraserbn@gmail\.com wrote:

On Wed\, Sep 25\, 2013 at 9:14 AM\, Craig A. Berry \craig\.a\.berry@gmail\.com wrote:

On Tue\, Sep 24\, 2013 at 8:32 AM\, Brian Fraser \fraserbn@gmail\.com wrote:

On Sat\, Sep 21\, 2013 at 11:47 PM\, Craig A. Berry \craig\.a\.berry@gmail\.com wrote:

On Sat\, Sep 21\, 2013 at 12:51 PM\, Father Chrysostomos via RT \perlbug\-followup@perl\.org wrote:

On Sat Sep 21 07:40:33 2013\, Hugmeir wrote:

I have done a smoke run of smoke-me/hugmeir/dup_glob_state\, results available at:

\<http://www.nntp.perl.org/group/perl.daily-build.reports/2013/09/msg151124.html>

Could I ask you to fetch the branch again and try that test?

Here ha go:

\<http://www.nntp.perl.org/group/perl.daily-build.reports/2013/09/msg151317.html>

The test failure I think you're interested in looks like:

$ perl ../ext/File-Glob/t/threads.t not ok 1 # Failed test at ../ext/File-Glob/t/threads.t line 44. # got: '1_file.' # expected: '1_file' not ok 2 - glob() state is cloned for new threads # Failed test 'glob() state is cloned for new threads' # at ../ext/File-Glob/t/threads.t line 48. # Structures begin differing at: # $got->[0] = '2_file.' # $expected->[0] = '2_file' not ok 3 - ..and for new threads inside threads # Failed test '..and for new threads inside threads' # at ../ext/File-Glob/t/threads.t line 59. # Structures begin differing at: # $got->[0] = '2_file.' # $expected->[0] = '2_file' not ok 4 - state doesn't leak from threads # Failed test 'state doesn't leak from threads' # at ../ext/File-Glob/t/threads.t line 66. # got: '2_file.' # expected: '2_file' 1..4 # Looks like you failed 4 tests of 4. %SYSTEM-F-ABORT\, abort $

And the gotcha is that on VMS\, all files have extensions\, even those that don't :-). In other words\, the trailing dot indicates a zero-length extension. The path of least resistance is to just put an explicit extension on those files:

--- ext/File-Glob/t/threads.t;-0 2013-09-22 05:26:48 -0500 +++ ext/File-Glob/t/threads.t 2013-09-25 07:08:29 -0500 @@ -28\,7 +28\,7 @@ use File::Glob qw(csh_glob); my($dir) = tempdir(CLEANUP => 1) or die "Could not create temporary directory";

-my @temp_files = qw(1_file 2_file 3_file); +my @temp_files = qw(1_file.tmp 2_file.tmp 3_file.tmp); for my $file (@temp_files) { open my $fh\, ">"\, File::Spec->catfile($dir\, $file) or die "Could not create file $dir/$file: $!"; [end]

which gets the test to pass:

$ perl ../ext/File-Glob/t/threads.t ok 1 ok 2 - glob() state is cloned for new threads ok 3 - ..and for new threads inside threads ok 4 - state doesn't leak from threads 1..4

Ooh\, thank you! I'll push the branch with those changes later today. (As a side note\, VMS actually sounds like a pretty fun system; I wish that I could just emulate it locally)

For some loose definition of "today." Fixed in facf34e on blead.

Actually it wasn't in facf34e\, but it now is in 43ed1b742e :-).

p5pRT commented 6 years ago

From @ppisar

Dne Wed\, 01 May 2013 04:55:31 -0700\, trast@inf.ethz.ch napsal(a):

It seems \<glob*> contains a race when used in parallel in threads. This test case reliably spews a few errors\, and then segfaults\, when I run it on perl v5.16.2 (stock opensuse 12.3):

It looks like the facf34ef4 fix for this issue broke "forks" module \https://rt.cpan.org/Public/Bug/Display.html?id=123248. A forked child crashes in threads->isthread() now.

Perl / perl5