[perl #119893] avoid waiting on pid 0

tonycoz · tonycoz · commit e9d373c4fc63 · 2013-10-17T15:43:09.000+11:00
When a filehandle is cloned into a standard handle, do_openn() copies
the pid from the original handle in PL_fdpid to the standard handle
and zeroes the entry for the original handle, so when the original
handle was closed Perl_my_pclose() would call wait4pid() with a pid of 0.

With v5.19.3-614-gd4c0274 I modified wait4pid(), perl's waitpid/wait4()
wrapper, to allow a pid of zero through to the actual system call when
available.

These combined so that following v5.19.3-614-gd4c0274 in some
circumstances closing the original handle would block by calling
waitpid(0, ...) or wait4(0, ...), which waits for any child process in
the same process group to terminate.

This commit changes Perl_my_pclose() to wait for the child only when
the stored pid is positive.
diff --git a/MANIFEST b/MANIFEST
@@ -4993,6 +4993,7 @@ time64.h			64 bit clean time.h (header)
 t/io/argv.t			See if ARGV stuff works
 t/io/binmode.t			See if binmode() works
 t/io/bom.t			See if scripts can start with a byte order mark
+t/io/closepid.t			See if close works for subprocesses
 t/io/crlf.t			See if :crlf works
 t/io/crlf_through.t		See if pipe passes data intact with :crlf
 t/io/data.t			See if DATA works
diff --git a/t/io/closepid.t b/t/io/closepid.t
@@ -0,0 +1,47 @@
+#!./perl
+
+BEGIN {
+    chdir 't' if -d 't';
+    @INC = '../lib';
+    require './test.pl';
+}
+
+if ($^O eq 'dos') {
+    skip_all("no multitasking");
+}
+
+plan tests => 3;
+watchdog(10, $^O eq 'MSWin32' ? "alarm" : '');
+
+use Config;
+$| = 1;
+$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/;
+
+SKIP:
+{
+    skip("Not relevant to $^O", 3)
+      if $^O eq "MSWin32" || $^O eq "VMS";
+    skip("only matters for waitpid or wait4", 3)
+      unless $Config{d_waitpid} || $Config{d_wait4};
+    # [perl #119893]
+    # 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" |/;
+    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");
+    # now PL_fdpids[fileno($fh1)] is zero and PL_fdpids[0] is
+    # the pid of the process created above, previously this would block
+    # internally on waitpid(0, ...)
+    ok(close($fh1), "close the original");
+    kill $killsig, $pid;
+    open STDIN, "<&", $savein;
+}
diff --git a/util.c b/util.c
@@ -2706,19 +2706,21 @@ Perl_my_pclose(pTHX_ PerlIO *ptr)
     bool close_failed;
     dSAVEDERRNO;
     const int fd = PerlIO_fileno(ptr);
+    bool should_wait;
+
+    svp = av_fetch(PL_fdpid,fd,TRUE);
+    pid = (SvTYPE(*svp) == SVt_IV) ? SvIVX(*svp) : -1;
+    SvREFCNT_dec(*svp);
+    *svp = NULL;
 
 #ifdef USE_PERLIO
     /* Find out whether the refcount is low enough for us to wait for the
        child proc without blocking. */
-    const bool should_wait = PerlIOUnix_refcnt(fd) == 1;
+    should_wait = PerlIOUnix_refcnt(fd) == 1 && pid > 0;
 #else
-    const bool should_wait = 1;
+    should_wait = pid > 0;
 #endif
 
-    svp = av_fetch(PL_fdpid,fd,TRUE);
-    pid = (SvTYPE(*svp) == SVt_IV) ? SvIVX(*svp) : -1;
-    SvREFCNT_dec(*svp);
-    *svp = NULL;
 #ifdef OS2
     if (pid == -1) {			/* Opened by popen. */
 	return my_syspclose(ptr);
