4dad76
From 35608a1658fe75c79ca53d96aea6cf7cb2a98615 Mon Sep 17 00:00:00 2001
4dad76
From: Tony Cook <tony@develop-help.com>
4dad76
Date: Thu, 9 May 2019 09:52:30 +1000
4dad76
Subject: [PATCH] (perl #122112) a simpler fix for pclose() aborted by a signal
4dad76
MIME-Version: 1.0
4dad76
Content-Type: text/plain; charset=UTF-8
4dad76
Content-Transfer-Encoding: 8bit
4dad76
4dad76
This change results in a zombie child process for the lifetime of
4dad76
the process, but I think that's the responsibility of the signal
4dad76
handler that aborted pclose().
4dad76
4dad76
We could add some magic to retry (and retry and retry) waiting on
4dad76
child process as we rewind (since there's no other way to remove
4dad76
the zombie), but the program has chosen implicitly to abort the
4dad76
wait() done by pclose() and it's best to honor that.
4dad76
4dad76
If we do choose to retry the wait() we might be blocking an attempt
4dad76
by the process to terminate, whether by exit() or die().
4dad76
4dad76
If a program does need more flexible handling there's always
4dad76
pipe()/fork()/exec() and/or the various event-driven frameworks on
4dad76
CPAN.
4dad76
4dad76
Signed-off-by: Petr Písař <ppisar@redhat.com>
4dad76
---
4dad76
 doio.c      | 12 +++++++++++-
4dad76
 t/io/pipe.t |  2 --
4dad76
 2 files changed, 11 insertions(+), 3 deletions(-)
4dad76
4dad76
diff --git a/doio.c b/doio.c
4dad76
index 0cc4e55404..05a06968dc 100644
4dad76
--- a/doio.c
4dad76
+++ b/doio.c
4dad76
@@ -1779,7 +1779,17 @@ Perl_io_close(pTHX_ IO *io, GV *gv, bool not_implicit, bool warn_on_fail)
4dad76
 
4dad76
     if (IoIFP(io)) {
4dad76
 	if (IoTYPE(io) == IoTYPE_PIPE) {
4dad76
-	    const int status = PerlProc_pclose(IoIFP(io));
4dad76
+            PerlIO *fh = IoIFP(io);
4dad76
+            int status;
4dad76
+
4dad76
+            /* my_pclose() can propagate signals which might bypass any code
4dad76
+               after the call here if the signal handler throws an exception.
4dad76
+               This would leave the handle in the IO object and try to close it again
4dad76
+               when the SV is destroyed on unwind or global destruction.
4dad76
+               So NULL it early.
4dad76
+            */
4dad76
+            IoOFP(io) = IoIFP(io) = NULL;
4dad76
+	    status = PerlProc_pclose(fh);
4dad76
 	    if (not_implicit) {
4dad76
 		STATUS_NATIVE_CHILD_SET(status);
4dad76
 		retval = (STATUS_UNIX == 0);
4dad76
diff --git a/t/io/pipe.t b/t/io/pipe.t
4dad76
index 1d01db6af6..fc3071300d 100644
4dad76
--- a/t/io/pipe.t
4dad76
+++ b/t/io/pipe.t
4dad76
@@ -255,9 +255,7 @@ close \$fh;
4dad76
 PROG
4dad76
     print $prog;
4dad76
     my $out = fresh_perl($prog, {});
4dad76
-    $::TODO = "not fixed yet";
4dad76
     cmp_ok($out, '!~', qr/refcnt/, "no exception from PerlIO");
4dad76
-    undef $::TODO;
4dad76
     # checks that that program did something rather than failing to
4dad76
     # compile
4dad76
     cmp_ok($out, '=~', qr/Died at/, "but we did get the exception from die");
4dad76
-- 
4dad76
2.20.1
4dad76