Skip Menu |

Date: Fri, 17 Jan 2014 18:36:04 -0500
To: perlbug [...] perl.org
Subject: [PATCH] t/io/closepid.t hangs for test timeout (50 seconds) under harness
CC: tony <tony [...] develop-help.com>
From: "Matthew Horsfall (alh)" <wolfsage [...] gmail.com>
Download (untitled) / with headers
text/plain 4.7k
This is a bug report for perl from wolfsage@gmail.com, generated with the help of perlbug 1.39 running under perl 5.14.2. ----------------------------------------------------------------- [Please describe your issue here] $ time ./perl harness io/closepid.t io/closepid.t .. ok All tests successful. Files=1, Tests=3, 50 wallclock secs ( 0.00 usr + 0.01 sys = 0.01 CPU) Result: PASS real 0m50.079s user 0m0.056s sys 0m0.020s With patch: $ time ./perl harness io/closepid.t io/closepid.t .. ok All tests successful. Files=1, Tests=3, 0 wallclock secs ( 0.01 usr 0.00 sys + 0.02 cusr 0.00 csys = 0.03 CPU) Result: PASS real 0m0.093s user 0m0.080s sys 0m0.012s The problem seems to be that the test is forking a perl process like so: my $pid = open my $fh1, qq/$perl -e "sleep 50" |/; Which starts two processes, 'sh -c "$perl -e \"sleep 50\""' and the perl -e... The pid from sh is returned, and later we try to clean up by sending it a HUP: kill $killsig, $pid; On my system, sh -c ".." doesn't exit on a HUP, but it does on a TERM. I've updated the test to use TERM instead, however I'm not sure if this violates the original test case. Tony, it looks like you added this in e9d373c4, can you comment? Thanks, -- Matthew Horsfall (alh) [Please do not change anything below this line] ----------------------------------------------------------------- --- Flags: category=core severity=low --- Site configuration information for perl 5.14.2: Configured by Debian Project at Mon Mar 18 19:16:26 UTC 2013. Summary of my perl5 (revision 5 version 14 subversion 2) configuration: Platform: osname=linux, osvers=2.6.42-37-generic, archname=x86_64-linux-gnu-thread-multi uname='linux batsu 2.6.42-37-generic #58-ubuntu smp thu jan 24 15:28:10 utc 2013 x86_64 x86_64 x86_64 gnulinux ' config_args='-Dusethreads -Duselargefiles -Dccflags=-DDEBIAN -Dcccdlflags=-fPIC -Darchname=x86_64-linux-gnu -Dprefix=/usr -Dprivlib=/usr/share/perl/5.14 -Darchlib=/usr/lib/perl/5.14 -Dvendorprefix=/usr -Dvendorlib=/usr/share/perl5 -Dvendorarch=/usr/lib/perl5 -Dsiteprefix=/usr/local -Dsitelib=/usr/local/share/perl/5.14.2 -Dsitearch=/usr/local/lib/perl/5.14.2 -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 -DDEBUGGING=-g -Doptimize=-O2 -Duseshrplib -Dlibperl=libperl.so.5.14.2 -des' 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 -DDEBIAN -fno-strict-aliasing -pipe -fstack-protector -I/usr/local/include -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64', optimize='-O2 -g', cppflags='-D_REENTRANT -D_GNU_SOURCE -DDEBIAN -fno-strict-aliasing -pipe -fstack-protector -I/usr/local/include' ccversion='', gccversion='4.6.3', 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 =' -fstack-protector -L/usr/local/lib' libpth=/usr/local/lib /lib/x86_64-linux-gnu /lib/../lib /usr/lib/x86_64-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.14.2 gnulibc_version='2.15' Dynamic Linking: dlsrc=dl_dlopen.xs, dlext=so, d_dlsymun=undef, ccdlflags='-Wl,-E' cccdlflags='-fPIC', lddlflags='-shared -O2 -g -L/usr/local/lib -fstack-protector' Locally applied patches: --- @INC for perl 5.14.2: /etc/perl /usr/local/lib/perl/5.14.2 /usr/local/share/perl/5.14.2 /usr/lib/perl5 /usr/share/perl5 /usr/lib/perl/5.14 /usr/share/perl/5.14 /usr/local/lib/site_perl . --- Environment for perl 5.14.2: HOME=/home/mhorsfall LANG=en_US.UTF-8 LANGUAGE (unset) LD_LIBRARY_PATH (unset) LOGDIR (unset) PATH=/home/mhorsfall/perl5/perlbrew/bin:/home/mhorsfall/bin:/home/mhorsfall/bin:/usr/lib/lightdm/lightdm:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin:/usr/games PERLBREW_BASHRC_VERSION=0.66 PERLBREW_HOME=/home/mhorsfall/.perlbrew PERLBREW_ROOT=/home/mhorsfall/perl5/perlbrew PERL_BADLANG (unset) SHELL=/bin/bash

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

RT-Send-CC: perl5-porters [...] perl.org
On Fri Jan 17 15:36:14 2014, alh wrote: Show quoted text
> $ time ./perl harness io/closepid.t > > io/closepid.t .. ok > All tests successful. > Files=1, Tests=3, 50 wallclock secs ( 0.00 usr + 0.01 sys = 0.01 > CPU) > Result: PASS > > real 0m50.079s > user 0m0.056s > sys 0m0.020s > > With patch: > > $ time ./perl harness io/closepid.t > io/closepid.t .. ok > All tests successful. > Files=1, Tests=3, 0 wallclock secs ( 0.01 usr 0.00 sys + 0.02 > cusr 0.00 csys = 0.03 CPU) > Result: PASS > > real 0m0.093s > user 0m0.080s > sys 0m0.012s > > > The problem seems to be that the test is forking a perl process like > so: > > my $pid = open my $fh1, qq/$perl -e "sleep 50" |/; > > Which starts two processes, 'sh -c "$perl -e \"sleep 50\""' and the > perl -e... > > The pid from sh is returned, and later we try to clean up by sending > it a HUP: > > kill $killsig, $pid; > > On my system, sh -c ".." doesn't exit on a HUP, but it does on a TERM. > > I've updated the test to use TERM instead, however I'm not sure if > this > violates the original test case. > > Tony, it looks like you added this in e9d373c4, can you comment?
The change in signal should make no difference to what test is trying to test for and I can reproduce the delay you see. Your change doesn't make any difference to the delay for me though: # blead is 3ac2191a2512af33d289fa09e823cb106596760d # closepid-delay is blead plus your patch tony@mars:.../perl2/t$ git checkout blead Switched to branch 'blead' tony@mars:.../perl2/t$ time ./perl harness io/closepid.t io/closepid.t .. ok All tests successful. Files=1, Tests=3, 50 wallclock secs ( 0.01 usr 0.00 sys + 0.01 cusr 0.01 csys = 0.03 CPU) Result: PASS real 0m50.124s user 0m0.112s sys 0m0.016s tony@mars:.../perl2/t$ git checkout closepid-delay Switched to branch 'closepid-delay' tony@mars:.../perl2/t$ git show commit 5077467e313c7cff7a4b951cb49c744f4a0e3125 Author: Matthew Horsfall <WolfSage@gmail.com> Date: Fri Jan 17 18:33:08 2014 -0500 Switch from HUP to TERM so test sub-process exits when asked for under harne 'make test' does not show the slow behaviour (t/TEST), only under TAP::Harness (t/harness) do we see it. diff --git a/t/io/closepid.t b/t/io/closepid.t index aa937f5..bdd8c8e 100644 --- a/t/io/closepid.t +++ b/t/io/closepid.t @@ -16,13 +16,13 @@ watchdog(10, $^O eq 'MSWin32' ? "alarm" : ''); use Config; $| = 1; $SIG{PIPE} = 'IGNORE'; -$SIG{HUP} = 'IGNORE' if $^O eq 'interix'; +$SIG{TERM} = 'IGNORE' if $^O eq 'interix'; my $perl = which_perl(); $perl .= qq[ "-I../lib"]; -my $killsig = 'HUP'; -$killsig = 1 unless $Config{sig_name} =~ /\bHUP\b/; +my $killsig = 'TERM'; +$killsig = 15 unless $Config{sig_name} =~ /\bTERM\b/; SKIP: { tony@mars:.../perl2/t$ time ./perl harness io/closepid.t io/closepid.t .. ok All tests successful. Files=1, Tests=3, 50 wallclock secs ( 0.01 usr 0.00 sys + 0.02 cusr 0.00 csys = 0.03 CPU) Result: PASS real 0m50.108s user 0m0.100s sys 0m0.008s A different approach is to not use the shell, please let me know if the attached fixes the problem for you. Tony
Subject: 0001-perl-121028-avoid-creating-a-shell-process.patch
From 4b0a73e22aa3e59e48b32ef2409ea89ed5c3f125 Mon Sep 17 00:00:00 2001 From: Tony Cook <tony@develop-help.com> Date: Mon, 20 Jan 2014 09:39:19 +1100 Subject: [perl #121028] avoid creating a shell process --- t/io/closepid.t | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/t/io/closepid.t b/t/io/closepid.t index aa937f5..b9e9e15 100644 --- a/t/io/closepid.t +++ b/t/io/closepid.t @@ -19,7 +19,6 @@ $SIG{PIPE} = 'IGNORE'; $SIG{HUP} = 'IGNORE' if $^O eq 'interix'; my $perl = which_perl(); -$perl .= qq[ "-I../lib"]; my $killsig = 'HUP'; $killsig = 1 unless $Config{sig_name} =~ /\bHUP\b/; @@ -34,7 +33,7 @@ SKIP: # close on the original of a popen handle dupped to a standard handle # would wait4pid(0, ...) open my $savein, "<&", \*STDIN; - my $pid = open my $fh1, qq/$perl -e "sleep 50" |/; + my $pid = open my $fh1, "-|", $perl, "-e", "sleep 50"; ok($pid, "open a pipe"); # at this point PL_fdpids[fileno($fh1)] is the pid of the new process ok(open(STDIN, "<&=", $fh1), "dup the pipe"); -- 1.7.10.4
From: "Matthew Horsfall (alh)" <wolfsage [...] gmail.com>
CC: Perl5 Porters <perl5-porters [...] perl.org>
Subject: Re: [perl #121028] [PATCH] t/io/closepid.t hangs for test timeout (50 seconds) under harness
To: perlbug-followup [...] perl.org
Date: Mon, 20 Jan 2014 09:25:44 -0500
Download (untitled) / with headers
text/plain 421b
On Sun, Jan 19, 2014 at 5:42 PM, Tony Cook via RT <perlbug-followup@perl.org> wrote: Show quoted text
> A different approach is to not use the shell, please let me know if the attached fixes the problem for you.
Your patch doesn't fix it for me - my perl doesn't exit on a SIGHUP. Perhaps your patch + SIGTERM, or some other way to signal to the child process that it's time to end? (Or send both signals?) -- Matthew Horsfall (alh)
Date: Mon, 20 Jan 2014 12:51:32 -0500
From: "Matthew Horsfall (alh)" <wolfsage [...] gmail.com>
CC: Perl5 Porters <perl5-porters [...] perl.org>
Subject: Re: [perl #121028] [PATCH] t/io/closepid.t hangs for test timeout (50 seconds) under harness
To: perlbug-followup [...] perl.org
Download (untitled) / with headers
text/plain 161b
Whatever the solution, t/io/openpid.t suffers the same problem and can be reduced to less than a second from its current 30 seconds. -- Matthew Horsfall (alh)
RT-Send-CC: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 695b
On Mon Jan 20 06:26:02 2014, alh wrote: Show quoted text
> On Sun, Jan 19, 2014 at 5:42 PM, Tony Cook via RT > <perlbug-followup@perl.org> wrote: >
> > A different approach is to not use the shell, please let me know if > > the attached fixes the problem for you.
> > Your patch doesn't fix it for me - my perl doesn't exit on a SIGHUP. > > Perhaps your patch + SIGTERM, or some other way to signal to the child > process that it's time to end? (Or send both signals?) > > -- Matthew Horsfall (alh)
What is the output of this command on your system? perl -MPOSIX=sigprocmask,SIG_SETMASK -E 'sigprocmask(SIG_SETMASK, undef, my $set = POSIX::SigSet->new); say join "", map $set->ismember($_), 1 .. 32' Leon
Date: Tue, 21 Jan 2014 09:16:13 -0500
To: perlbug-followup [...] perl.org
Subject: Re: [perl #121028] [PATCH] t/io/closepid.t hangs for test timeout (50 seconds) under harness
CC: Perl5 Porters <perl5-porters [...] perl.org>
From: "Matthew Horsfall (alh)" <wolfsage [...] gmail.com>
Download (untitled) / with headers
text/plain 1.1k
On Mon, Jan 20, 2014 at 8:10 PM, Leon Timmermans via RT <perlbug-followup@perl.org> wrote: Show quoted text
> On Mon Jan 20 06:26:02 2014, alh wrote:
>> On Sun, Jan 19, 2014 at 5:42 PM, Tony Cook via RT >> <perlbug-followup@perl.org> wrote: >>
>> > A different approach is to not use the shell, please let me know if >> > the attached fixes the problem for you.
>> >> Your patch doesn't fix it for me - my perl doesn't exit on a SIGHUP. >> >> Perhaps your patch + SIGTERM, or some other way to signal to the child >> process that it's time to end? (Or send both signals?) >> >> -- Matthew Horsfall (alh)
> > What is the output of this command on your system? > > perl -MPOSIX=sigprocmask,SIG_SETMASK -E 'sigprocmask(SIG_SETMASK, undef, my $set = POSIX::SigSet->new); say join "", map $set->ismember($_), 1 .. 32'
mhorsfall@tworivers:~$ perl -MPOSIX=sigprocmask,SIG_SETMASK -E 'sigprocmask(SIG_SETMASK, undef, my $set = POSIX::SigSet->new); say join "", map $set->ismember($_), 1 .. 32' 00000000000000000000000000000000 mhorsfall@tworivers:~$ uname -a Linux tworivers 3.8.0-31-generic #46~precise1-Ubuntu SMP Wed Sep 11 18:21:16 UTC 2013 x86_64 x86_64 x86_64 GNU/Linux -- Matthew Horsfall (alh)
Subject: Re: [perl #121028] [PATCH] t/io/closepid.t hangs for test timeout (50 seconds) under harness
To: perlbug-followup [...] perl.org
From: "Matthew Horsfall (alh)" <wolfsage [...] gmail.com>
CC: Perl5 Porters <perl5-porters [...] perl.org>
Date: Tue, 21 Jan 2014 09:30:30 -0500
Download (untitled) / with headers
text/plain 723b
On Tue, Jan 21, 2014 at 9:16 AM, Matthew Horsfall (alh) <wolfsage@gmail.com> wrote: Show quoted text
> > mhorsfall@tworivers:~$ perl -MPOSIX=sigprocmask,SIG_SETMASK -E > 'sigprocmask(SIG_SETMASK, undef, my $set = POSIX::SigSet->new); say > join "", map $set->ismember($_), 1 .. 32' > 00000000000000000000000000000000 > > mhorsfall@tworivers:~$ uname -a > Linux tworivers 3.8.0-31-generic #46~precise1-Ubuntu SMP Wed Sep 11 > 18:21:16 UTC 2013 x86_64 x86_64 x86_64 GNU/Linux
Oddly, my other ubuntu box (Which is ubuntu server instead of ubuntu desktop) behaves correctly: mhorsfall@dory:~$ uname -a Linux dory 3.8.0-29-generic #42~precise1-Ubuntu SMP Wed Aug 14 16:19:23 UTC 2013 x86_64 x86_64 x86_64 GNU/Linux -- Matthew Horsfall (alh)
Date: Tue, 21 Jan 2014 15:37:21 +0100
To: "Matthew Horsfall (alh)" <wolfsage [...] gmail.com>
Subject: Re: [perl #121028] [PATCH] t/io/closepid.t hangs for test timeout (50 seconds) under harness
CC: Father Chrysostomos via RT <perlbug-followup [...] perl.org>, Perl5 Porters <perl5-porters [...] perl.org>
From: Leon Timmermans <fawaka [...] gmail.com>
Download (untitled) / with headers
text/plain 572b
On Tue, Jan 21, 2014 at 3:16 PM, Matthew Horsfall (alh) <wolfsage@gmail.com> wrote:
Show quoted text

> What is the output of this command on your system?
>
> perl -MPOSIX=sigprocmask,SIG_SETMASK -E 'sigprocmask(SIG_SETMASK, undef, my $set = POSIX::SigSet->new); say join "", map $set->ismember($_), 1 .. 32'

mhorsfall@tworivers:~$ perl -MPOSIX=sigprocmask,SIG_SETMASK -E
'sigprocmask(SIG_SETMASK, undef, my $set = POSIX::SigSet->new); say
join "", map $set->ismember($_), 1 .. 32'
00000000000000000000000000000000

And how about « perl -E 'say $SIG{HUP}' » ?

Leon
Subject: Re: [perl #121028] [PATCH] t/io/closepid.t hangs for test timeout (50 seconds) under harness
To: Leon Timmermans <fawaka [...] gmail.com>
From: "Matthew Horsfall (alh)" <wolfsage [...] gmail.com>
CC: Father Chrysostomos via RT <perlbug-followup [...] perl.org>, Perl5 Porters <perl5-porters [...] perl.org>
Date: Tue, 21 Jan 2014 10:08:44 -0500
Download (untitled) / with headers
text/plain 249b
On Tue, Jan 21, 2014 at 9:37 AM, Leon Timmermans <fawaka@gmail.com> wrote: Show quoted text
> And how about « perl -E 'say $SIG{HUP}' » ?
mhorsmhorsfall@tworivers:~/perl-1$ ./perl -Ilib -E 'say $SIG{HUP}' IGNORE What's going on here? -- Matthew Horsfall (alh)
Date: Tue, 21 Jan 2014 16:20:25 +0100
CC: Father Chrysostomos via RT <perlbug-followup [...] perl.org>, Perl5 Porters <perl5-porters [...] perl.org>
From: Leon Timmermans <fawaka [...] gmail.com>
To: "Matthew Horsfall (alh)" <wolfsage [...] gmail.com>
Subject: Re: [perl #121028] [PATCH] t/io/closepid.t hangs for test timeout (50 seconds) under harness
Download (untitled) / with headers
text/plain 439b
On Tue, Jan 21, 2014 at 4:08 PM, Matthew Horsfall (alh) <wolfsage@gmail.com> wrote:
Show quoted text
On Tue, Jan 21, 2014 at 9:37 AM, Leon Timmermans <fawaka@gmail.com> wrote:
> And how about « perl -E 'say $SIG{HUP}' » ?

mhorsmhorsfall@tworivers:~/perl-1$ ./perl -Ilib -E 'say $SIG{HUP}'
IGNORE

What's going on here?

Something sets up your environment to ignore SIGHUP by default. Possibly a «trap "" HUP» in you shell profile.

Leon
RT-Send-CC: perl5-porters [...] perl.org
On Mon Jan 20 06:26:02 2014, alh wrote: Show quoted text
> On Sun, Jan 19, 2014 at 5:42 PM, Tony Cook via RT > <perlbug-followup@perl.org> wrote: >
> > A different approach is to not use the shell, please let me know if > > the attached fixes the problem for you.
> > Your patch doesn't fix it for me - my perl doesn't exit on a SIGHUP. > > Perhaps your patch + SIGTERM, or some other way to signal to the child > process that it's time to end? (Or send both signals?)
The attached patch allows closepid() to finish quickly for me in a shell with C< trap "" HUP >. This makes sure that $SIG{HUP} is DEFAULT before we expect it to work, though perhaps it should just skip when $SIG{HUP} and complain your system is broken. Preventing openpid.t from delaying processing would require similar changes I think - using 3+ arg open for the pipe opens (to avoid the extra processes) and preventing $SIG{HUP} being ignored. Note that the problem in both cases isn't that (open|close)pid.t is waiting, but that harness is waiting for the stdout handle from the children to be closed. Tony
Subject: 0001-perl-121028-avoid-creating-a-shell-process.patch
From 0bc4d3b37fc2ae0ffd41dc5d358e50f33da7ff33 Mon Sep 17 00:00:00 2001 From: Tony Cook <tony@develop-help.com> Date: Wed, 22 Jan 2014 15:14:59 +1100 Subject: [PATCH] [perl #121028] avoid creating a shell process --- t/io/closepid.t | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/t/io/closepid.t b/t/io/closepid.t index aa937f5..a90db68 100644 --- a/t/io/closepid.t +++ b/t/io/closepid.t @@ -16,10 +16,11 @@ watchdog(10, $^O eq 'MSWin32' ? "alarm" : ''); use Config; $| = 1; $SIG{PIPE} = 'IGNORE'; +# work around a shell set to ignore HUP +$SIG{HUP} = 'DEFAULT'; $SIG{HUP} = 'IGNORE' if $^O eq 'interix'; my $perl = which_perl(); -$perl .= qq[ "-I../lib"]; my $killsig = 'HUP'; $killsig = 1 unless $Config{sig_name} =~ /\bHUP\b/; @@ -34,7 +35,7 @@ SKIP: # close on the original of a popen handle dupped to a standard handle # would wait4pid(0, ...) open my $savein, "<&", \*STDIN; - my $pid = open my $fh1, qq/$perl -e "sleep 50" |/; + my $pid = open my $fh1, "-|", $perl, "-e", "sleep 50"; ok($pid, "open a pipe"); # at this point PL_fdpids[fileno($fh1)] is the pid of the new process ok(open(STDIN, "<&=", $fh1), "dup the pipe"); -- 1.7.10.4
To: perlbug-followup [...] perl.org
Subject: Re: [perl #121028] [PATCH] t/io/closepid.t hangs for test timeout (50 seconds) under harness
CC: Perl5 Porters <perl5-porters [...] perl.org>
From: "Matthew Horsfall (alh)" <wolfsage [...] gmail.com>
Date: Fri, 24 Jan 2014 08:07:40 -0500
Download (untitled) / with headers
text/plain 855b
On Tue, Jan 21, 2014 at 11:46 PM, Tony Cook via RT <perlbug-followup@perl.org> wrote: Show quoted text
> > This makes sure that $SIG{HUP} is DEFAULT before we expect it to work, though perhaps it should just skip when $SIG{HUP} and complain your system is broken. > > Preventing openpid.t from delaying processing would require similar changes I think - using 3+ arg open for the pipe opens (to avoid the extra processes) and preventing $SIG{HUP} being ignored. > > Note that the problem in both cases isn't that (open|close)pid.t is waiting, but that harness is waiting for the stdout handle from the children to be closed.
Awesome, this works great for me. How do you feel about applying this to other places where HUP/open()/kill are combined for this sort of functionality? If you're busy I'd be willing to throw this together. Thanks, -- Matthew Horsfall (alh)
RT-Send-CC: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 1.5k
On Fri Jan 24 05:08:05 2014, alh wrote: Show quoted text
> On Tue, Jan 21, 2014 at 11:46 PM, Tony Cook via RT > <perlbug-followup@perl.org> wrote:
> > > > This makes sure that $SIG{HUP} is DEFAULT before we expect it to > > work, though perhaps it should just skip when $SIG{HUP} and complain > > your system is broken. > > > > Preventing openpid.t from delaying processing would require similar > > changes I think - using 3+ arg open for the pipe opens (to avoid the > > extra processes) and preventing $SIG{HUP} being ignored. > > > > Note that the problem in both cases isn't that (open|close)pid.t is > > waiting, but that harness is waiting for the stdout handle from the > > children to be closed.
> > Awesome, this works great for me. > > How do you feel about applying this to other places where > HUP/open()/kill are combined for this sort of functionality? > > If you're busy I'd be willing to throw this together.
The attached patch allows openpid.t to finish quickly under harness on Linux - *but* fails horribly on Win32: C:\Users\tony\dev\perl\git\perl\t>.\perl io\openpid.t 1..10 List form of pipe open not implemented at io\openpid.t line 56. # Looks like you planned 10 tests but ran 0. Implementing list form pipes on Win32 is complicated by there being no standard command-line argument parsing mechanism - apparently VB for example uses different rules to the CRT. This isn't a problem for closepid.t, since that test is skipped on Win32. An option might be to implement list form pipes for Win32, but I'm not sure I want to jump down that rabbit hole. Tony
Subject: 0001-perl-121028-avoid-creating-a-shell-process.patch
From 0fd13b4b4422d848e9b1dcaacf8800a17a1f7cdb Mon Sep 17 00:00:00 2001 From: Tony Cook <tony@develop-help.com> Date: Tue, 28 Jan 2014 15:52:22 +1100 Subject: [PATCH] [perl #121028] avoid creating a shell process --- t/io/closepid.t | 5 +++-- t/io/openpid.t | 20 +++++++++++++++----- 2 files changed, 18 insertions(+), 7 deletions(-) diff --git a/t/io/closepid.t b/t/io/closepid.t index aa937f5..a90db68 100644 --- a/t/io/closepid.t +++ b/t/io/closepid.t @@ -16,10 +16,11 @@ watchdog(10, $^O eq 'MSWin32' ? "alarm" : ''); use Config; $| = 1; $SIG{PIPE} = 'IGNORE'; +# work around a shell set to ignore HUP +$SIG{HUP} = 'DEFAULT'; $SIG{HUP} = 'IGNORE' if $^O eq 'interix'; my $perl = which_perl(); -$perl .= qq[ "-I../lib"]; my $killsig = 'HUP'; $killsig = 1 unless $Config{sig_name} =~ /\bHUP\b/; @@ -34,7 +35,7 @@ SKIP: # close on the original of a popen handle dupped to a standard handle # would wait4pid(0, ...) open my $savein, "<&", \*STDIN; - my $pid = open my $fh1, qq/$perl -e "sleep 50" |/; + my $pid = open my $fh1, "-|", $perl, "-e", "sleep 50"; ok($pid, "open a pipe"); # at this point PL_fdpids[fileno($fh1)] is the pid of the new process ok(open(STDIN, "<&=", $fh1), "dup the pipe"); diff --git a/t/io/openpid.t b/t/io/openpid.t index 946fa5e..de75648 100644 --- a/t/io/openpid.t +++ b/t/io/openpid.t @@ -23,11 +23,15 @@ watchdog(15, $^O eq 'MSWin32' ? "alarm" : ''); use Config; $| = 1; $SIG{PIPE} = 'IGNORE'; +# reset the handler in case the shell has set a broken default +$SIG{HUP} = 'DEFAULT'; $SIG{HUP} = 'IGNORE' if $^O eq 'interix'; my $perl = which_perl(); $perl .= qq[ "-I../lib"]; +my @perl = ( which_perl(), "-I../lib" ); + # # commands run 4 perl programs. Two of these programs write a # short message to STDOUT and exit. Two of these programs @@ -35,16 +39,22 @@ $perl .= qq[ "-I../lib"]; # the other reader reads one line, waits a few seconds and then # exits to test the waitpid function. # -$cmd1 = qq/$perl -e "\$|=1; print qq[first process\\n]; sleep 30;"/; -$cmd2 = qq/$perl -e "\$|=1; print qq[second process\\n]; sleep 30;"/; +# Using 4+ arg open for the children that sleep so that that we're +# killing the perl process instead of an intermediate shell, this +# allows harness to see the file handles closed sooner. I didn't +# convert them all since I wanted 3-arg open to continue to be +# exercised here. +# +@cmd1 = ( @perl, "-e", "\$|=1; print qq[first process\\n]; sleep 30;" ); +@cmd2 = ( @perl, "-e", "\$|=1; print qq[second process\\n]; sleep 30;" ); $cmd3 = qq/$perl -e "print <>;"/; # hangs waiting for end of STDIN $cmd4 = qq/$perl -e "print scalar <>;"/; -#warn "#$cmd1\n#$cmd2\n#$cmd3\n#$cmd4\n"; +#warn "#@cmd1\n#@cmd2\n#$cmd3\n#$cmd4\n"; # start the processes -ok( $pid1 = open(FH1, "$cmd1 |"), 'first process started'); -ok( $pid2 = open(FH2, "$cmd2 |"), ' second' ); +ok( $pid1 = open(FH1, "-|", @cmd1), 'first process started'); +ok( $pid2 = open(FH2, "-|", @cmd2), ' second' ); { no warnings 'once'; ok( $pid3 = open(FH3, "| $cmd3"), ' third' ); -- 1.7.10.4
Subject: Re: [perl #121028] [PATCH] t/io/closepid.t hangs for test timeout (50 seconds) under harness
To: Father Chrysostomos via RT <perlbug-followup [...] perl.org>
From: Leon Timmermans <fawaka [...] gmail.com>
CC: Perl5 Porters <perl5-porters [...] perl.org>
Date: Thu, 30 Jan 2014 01:51:25 +0100
Download (untitled) / with headers
text/plain 804b
On Tue, Jan 28, 2014 at 6:48 AM, Tony Cook via RT <perlbug-followup@perl.org> wrote:
Show quoted text
C:\Users\tony\dev\perl\git\perl\t>.\perl io\openpid.t
1..10
List form of pipe open not implemented at io\openpid.t line 56.
# Looks like you planned 10 tests but ran 0.

Implementing list form pipes on Win32 is complicated by there being no standard command-line argument parsing mechanism - apparently VB for example uses different rules to the CRT.

This isn't a problem for closepid.t, since that test is skipped on Win32.

An option might be to implement list form pipes for Win32, but I'm not sure I want to jump down that rabbit hole.

I think that would be highly desirable anyway. I'd second only to unicode paths in "parts of perl where our system interfaces suck on Windows".

Leon
RT-Send-CC: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 567b
On Tue Jan 21 20:46:07 2014, tonyc wrote: Show quoted text
> The attached patch allows closepid() to finish quickly for me in a > shell with C< trap "" HUP >. > > This makes sure that $SIG{HUP} is DEFAULT before we expect it to work, > though perhaps it should just skip when $SIG{HUP} and complain your > system is broken.
I've applied the closepid.t change to blead as af1f7e283108d440048b37ea8707071df065b30e. I don't think it needs a perldelta entry. I'll leave this ticket open until list form pipe is available on Win32 and a similar change can be made to io/openpid.t Tony
RT-Send-CC: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 710b
On Mon, 03 Feb 2014 15:41:46 -0800, tonyc wrote: Show quoted text
> On Tue Jan 21 20:46:07 2014, tonyc wrote:
> > The attached patch allows closepid() to finish quickly for me in a > > shell with C< trap "" HUP >. > > > > This makes sure that $SIG{HUP} is DEFAULT before we expect it to > > work, > > though perhaps it should just skip when $SIG{HUP} and complain your > > system is broken.
> > I've applied the closepid.t change to blead as > af1f7e283108d440048b37ea8707071df065b30e. > > I don't think it needs a perldelta entry. > > I'll leave this ticket open until list form pipe is available on Win32 > and a similar change can be made to io/openpid.t
Which was done in b6811f8d3a5c4826ad03be7b9dc69f5f3dc939be. Tony


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

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