IPC::Open3::open3() shouldn't fail if any of *STD{IN,OUT,ERR} are localized.

Previously it would fail on Win32, because spawn_with_handles() would attempt
to duplicate all three, ignoring failures at this point, but then report
failure to close any as a fatal error, even if this was because the earlier
dup-ing had failed.

Also avoid a warning in the *nix code path in open3() if STDERR is localized
(and hence fileno STDERR is undefined).

2 files changed, 32 insertions, 5 deletions

diff --git a/ext/IPC-Open3/lib/IPC/Open3.pm b/ext/IPC-Open3/lib/IPC/Open3.pm
index 4513fffc3a..4396603144 100644
--- a/ext/IPC-Open3/lib/IPC/Open3.pm
+++ b/ext/IPC-Open3/lib/IPC/Open3.pm
@@ -283,7 +283,8 @@ sub _open3 {
 			xopen \*STDERR, ">&=" . fileno $kid_err;
 		    }
 		} else {
-		    xopen \*STDERR, ">&STDOUT" if fileno(STDERR) != fileno(STDOUT);
+		    xopen \*STDERR, ">&STDOUT"
+			if defined fileno(STDERR) && fileno(STDERR) != fileno(STDOUT);
 		}
 		return 0 if ($cmd[0] eq '-');
 		exec @cmd or do {
@@ -388,7 +389,7 @@ sub spawn_with_handles {
 
     foreach $fd (@$fds) {
 	$fd->{tmp_copy} = IO::Handle->new_from_fd($fd->{handle}, $fd->{mode});
-	$saved{fileno $fd->{handle}} = $fd->{tmp_copy};
+	$saved{fileno $fd->{handle}} = $fd->{tmp_copy} if $fd->{tmp_copy};
     }
     foreach $fd (@$fds) {
 	bless $fd->{handle}, 'IO::Handle'
@@ -431,9 +432,12 @@ sub spawn_with_handles {
 	push @errs, "IO::Pipe: Can't spawn-NOWAIT: $!" if !$pid || $pid < 0;
     }
 
-    foreach $fd (@$fds) {
+    # Do this in reverse, so that STDERR is restored first:
+    foreach $fd (reverse @$fds) {
 	$fd->{handle}->fdopen($fd->{tmp_copy}, $fd->{mode});
-	$fd->{tmp_copy}->close or croak "Can't close: $!";
+    }
+    foreach (values %saved) {
+	$_->close or croak "Can't close: $!";
     }
     croak join "\n", @errs if @errs;
     return $pid;
diff --git a/ext/IPC-Open3/t/IPC-Open3.t b/ext/IPC-Open3/t/IPC-Open3.t
index 79dd9367c0..0ecb841b13 100644
--- a/ext/IPC-Open3/t/IPC-Open3.t
+++ b/ext/IPC-Open3/t/IPC-Open3.t
@@ -14,7 +14,7 @@ BEGIN {
 }
 
 use strict;
-use Test::More tests => 25;
+use Test::More tests => 37;
 
 use IO::Handle;
 use IPC::Open3;
@@ -159,3 +159,26 @@ $TB->current_test($test);
 $pid = eval { open3 'WRITE', '', 'ERROR', '/non/existent/program'; };
 like($@, qr/^open3: Modification of a read-only value attempted at /,
      'open3 faults read-only parameters correctly') or do {waitpid $pid, 0};
+
+foreach my $handle (qw (DUMMY STDIN STDOUT STDERR)) {
+    local $::{$handle};
+    my $out = IO::Handle->new();
+    my $pid = eval {
+	local $SIG{__WARN__} = sub {
+	    open my $fh, '>/dev/tty';
+	    return if "@_" =~ m!^Use of uninitialized value \$fd.*IO/Handle\.pm!;
+	    print $fh "@_";
+	    die @_
+	};
+	open3 undef, $out, undef, $perl, '-le', "print q _# ${handle}_"
+    };
+    is($@, '', "No errors with localised $handle");
+    cmp_ok($pid, '>', 0, "Got a pid with localised $handle");
+    if ($handle eq 'STDOUT') {
+	is(<$out>, undef, "Expected no output with localised $handle");
+    } else {
+	like(<$out>, qr/\A# $handle\r?\n\z/,
+	     "Expected output with localised $handle");
+    }
+    waitpid $pid, 0;
+}