Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

IPC::Open3 does untie STDOUT but does not untie STDERR, causing issue in FCGI usage #13276

Closed
p5pRT opened this issue Sep 16, 2013 · 32 comments
Closed

Comments

@p5pRT
Copy link

p5pRT commented Sep 16, 2013

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

Searchable as RT119843$

@p5pRT
Copy link
Author

p5pRT commented Sep 16, 2013

From ailin@devio.us

This is a bug report for perl from ailin@​devio.us,
generated with the help of perlbug 1.39 running under perl 5.14.2.


I am trying to use IPC​::Open3 to run a program from within a FCGI
application (Movable Type in this case). The error I get is "Not a
GLOB reference". This happens when trying to close the original STDERR
for redirection.

I have confirmed that the issue is still in the Perl 5.19.3 IPC​::Open3
source code and the fix is very trivial. Just add

  untie *STDERR;

to the same place as untie *STDOUT;

Thanks,
Nei (Ailin Nemui)



Flags​:
  category=library
  severity=medium
  module=IPC​::Open3


@p5pRT
Copy link
Author

p5pRT commented Sep 17, 2013

From @jkeenan

On Mon Sep 16 01​:45​:59 2013, ailin@​devio.us wrote​:

This is a bug report for perl from ailin@​devio.us,
generated with the help of perlbug 1.39 running under perl 5.14.2.

-----------------------------------------------------------------
I am trying to use IPC​::Open3 to run a program from within a FCGI
application (Movable Type in this case). The error I get is "Not a
GLOB reference". This happens when trying to close the original STDERR
for redirection.

I have confirmed that the issue is still in the Perl 5.19.3 IPC​::Open3
source code and the fix is very trivial. Just add

untie *STDERR;

to the same place as untie *STDOUT;

Thanks,
Nei (Ailin Nemui)

Is the attached the change you are recommending?

Thank you very much.
Jim Keenan

@p5pRT
Copy link
Author

p5pRT commented Sep 17, 2013

From @jkeenan

119843-ipc-open3.diff

@p5pRT
Copy link
Author

p5pRT commented Sep 17, 2013

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

@p5pRT
Copy link
Author

p5pRT commented Sep 19, 2013

From ailin@devio.us

On Mon, 2013-09-16 at 19​:03 -0700, James E Keenan via RT wrote​:

Is the attached the change you are recommending?

Thanks for putting my meaning into a diff file, this is exactly what I
was trying to say.

@p5pRT
Copy link
Author

p5pRT commented Sep 20, 2013

From @jkeenan

On Wed Sep 18 21​:48​:00 2013, ailin@​devio.us wrote​:

On Mon, 2013-09-16 at 19​:03 -0700, James E Keenan via RT wrote​:

Is the attached the change you are recommending?

Thanks for putting my meaning into a diff file, this is exactly what I
was trying to say.

Okay, while I've written the patch, I take no position on its merits, as
I have never used IPC​::Open3 myself. So we need to get some feedback
from contributors to the list on its merits.

Thank you very much.
Jim Keenan

@p5pRT
Copy link
Author

p5pRT commented Sep 20, 2013

From @ikegami

On Thu, Sep 19, 2013 at 9​:48 PM, James E Keenan via RT <
perlbug-followup@​perl.org> wrote​:

On Wed Sep 18 21​:48​:00 2013, ailin@​devio.us wrote​:

On Mon, 2013-09-16 at 19​:03 -0700, James E Keenan via RT wrote​:

Is the attached the change you are recommending?

Thanks for putting my meaning into a diff file, this is exactly what I
was trying to say.

Okay, while I've written the patch, I take no position on its merits, as
I have never used IPC​::Open3 myself. So we need to get some feedback
from contributors to the list on its merits.

C<< croak >> can be called after the point where you added C<< untie
*STDERR; >>. That's surely why C<< untie *STDERR; >> is not present along
with C<< untie *STDIN; >> and C<< untie *STDOUT; >>.

However, since the time when C<< untie *STDERR; >> was omitted because that
STDERR was still used, exception handling has been added such that nothing
writes to that STDERR after that point in the program. (Any exception
thrown is caught and transmitted to the parent over a pipe where it is
rethrown.)

As such, adding C<< untie *STDERR; >> looks safe to me.

@p5pRT
Copy link
Author

p5pRT commented Sep 22, 2013

From @jkeenan

On Wed Sep 18 21​:48​:00 2013, ailin@​devio.us wrote​:

On Mon, 2013-09-16 at 19​:03 -0700, James E Keenan via RT wrote​:

Is the attached the change you are recommending?

Thanks for putting my meaning into a diff file, this is exactly what I
was trying to say.

Can you write a unit test for this?

I am reluctant to take the "let's apply it to blead and see what breaks
on CPAN" approach for a module with which I have little experience.

Thank you very much.
Jim Keenan

@p5pRT
Copy link
Author

p5pRT commented Nov 19, 2013

From @jkeenan

On Sun Sep 22 10​:10​:25 2013, jkeenan wrote​:

On Wed Sep 18 21​:48​:00 2013, ailin@​devio.us wrote​:

On Mon, 2013-09-16 at 19​:03 -0700, James E Keenan via RT wrote​:

Is the attached the change you are recommending?

Thanks for putting my meaning into a diff file, this is exactly what I
was trying to say.

Can you write a unit test for this?

I am reluctant to take the "let's apply it to blead and see what breaks
on CPAN" approach for a module with which I have little experience.

Thank you very much.
Jim Keenan

I'm taking this ticket for the purpose of rejecting it within 7 days, as no unit test has been supplied.

Thank you very much.
Jim Keenan

@p5pRT
Copy link
Author

p5pRT commented Nov 21, 2013

From @jkeenan

On Mon Nov 18 16​:34​:18 2013, jkeenan wrote​:

I'm taking this ticket for the purpose of rejecting it within 7 days,
as no unit test has been supplied.

Marking ticket stalled.

@p5pRT
Copy link
Author

p5pRT commented Nov 21, 2013

@jkeenan - Status changed from 'open' to 'stalled'

@p5pRT
Copy link
Author

p5pRT commented Nov 21, 2013

From ailin@devio.us

On Mon, 2013-11-18 at 16​:34 -0800, James E Keenan via RT wrote​:

On Sun Sep 22 10​:10​:25 2013, jkeenan wrote​:

On Wed Sep 18 21​:48​:00 2013, ailin@​devio.us wrote​:

On Mon, 2013-09-16 at 19​:03 -0700, James E Keenan via RT wrote​:

Is the attached the change you are recommending?

Thanks for putting my meaning into a diff file, this is exactly what I
was trying to say.

Can you write a unit test for this?

I am reluctant to take the "let's apply it to blead and see what breaks
on CPAN" approach for a module with which I have little experience.

Thank you very much.
Jim Keenan

I'm taking this ticket for the purpose of rejecting it within 7 days, as no unit test has been supplied.

Thank you very much.
Jim Keenan

Hi,

I'm having trouble coming up with a unit test, as I am not an expert in
neither tying file handles nor unit tests. However I have boiled it down
to the following script which you can run under a FCGI SERVER.
Unfortunately, I could not locate any fcgi server implementation in
Perl, so you will have to use a fcgi server such as lighttpd. Here is a
suitable lighttpd configuration file​:

server.document-root = "/var/www"
server.port = 8082
server.errorlog = "error.log"
server.modules += ( "mod_fastcgi" )
fastcgi.debug = 1
$HTTP["url"] =~ "^/[^/]*$" {
  fastcgi.server = (
  "/" => ((
  "socket" => "/tmp/myapp.sock",
  "bin-path" => "/path/to/script.pl",
  "check-local" => "disable",
  )),
  )
}

and here is the test script; it dies spectacularly when trying to open3
the command with a white screen of death in the browser​:

#!/usr/bin/perl
use strict;
use warnings;
use FCGI;
use Symbol qw(gensym);

my $count = 0;
my $request = FCGI​::Request;

while ($request->Accept >= 0) {
  print "Content-type​: text/plain\r\n\r\n";
  require IPC​::Open3;
  my($wtr, $rdr, $err);
  #$err = gensym;
  my $pid = IPC​::Open3​::open3($wtr, $rdr, $err,
  $^X, '-e', 'print "hallo"');

  waitpid( $pid, 0 );
  print "error was​: ", <$err>, "\n";
  print "stdout was​: ", <$rdr>, "\n";
}

You have to open http​://localhost​:8082/xxx in your browser to run the
script.

Here is a stack trace as far as I could produce them​:

<white screen of death in browser>
FCGI​::Stream​::OPEN('FCGI​::Stream=SCALAR(0x2105480)', '>&STDOUT') called at perl5.16.2/IPC/Open3.pm line 160
IPC​::Open3​::xopen('GLOB(0x2105240)', '>&STDOUT') called at perl5.16.2/IPC/Open3.pm line 260
eval {...} called at perl5.16.2/IPC/Open3.pm line 277
IPC​::Open3​::_open3('open3', 'GLOB(0x2105708)', 'GLOB(0x2105858)', undef, '/usr/bin/perl', '-e', 'print "hallo"') called at perl5.16.2/IPC/Open3.pm line 350
IPC​::Open3​::open3('GLOB(0x2105708)', 'GLOB(0x2105858)', undef, '/usr/bin/perl', '-e', 'print "hallo"') called at perl5.16.2/IPC/Open3.pm line 304.
IPC​::Open3​::_open3('open3', 'GLOB(0x2105708)', 'GLOB(0x2105858)', undef, '/usr/bin/perl', '-e', 'print "hallo"') called at perl5.16.2/IPC/Open3.pm line 350
IPC​::Open3​::open3('GLOB(0x2105708)', 'GLOB(0x2105858)', undef, '/usr/bin/perl', '-e', 'print "hallo"') called at script.pl

Modifying IPC​::Open3 to untie STDERR fixes this problem consistently, so
I still have hopes that you could consider this fix.

Care,
Nei

@p5pRT
Copy link
Author

p5pRT commented Nov 21, 2013

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

@p5pRT
Copy link
Author

p5pRT commented Jun 6, 2014

From dtikhonov@yahoo.com

On Thu Sep 19 21​:18​:51 2013, ikegami@​adaelis.com wrote​:

C<< croak >> can be called after the point where you added C<< untie
*STDERR; >>. That's surely why C<< untie *STDERR; >> is not present along
with C<< untie *STDIN; >> and C<< untie *STDOUT; >>.

That code was added ten years ago (see RT#27986) and there are no comments as to why STDERR is not untied.

I came up with a small script that illustrates the problem, I think it is easy enough to convert into a unit test (see attachment). I think that this simple code should not fail.

@p5pRT
Copy link
Author

p5pRT commented Jun 6, 2014

From dtikhonov@yahoo.com

# Test to illustrate RT#119843 in IPC​::Open3

use strict;
use warnings;

use IPC​::Open3 qw(open3);

{
  package My​::Tied​::FH; # This simply prints to STDOUT
  sub TIEHANDLE {
  my $class = shift;
  bless {}, ref($class) || $class
  }
  sub PRINT {
  my $self = shift;
  print "TIE​: ", @​_;
  }
}

tie *STDERR, 'My​::Tied​::FH';

my $pid = open3(my $wtr, my $rdr, undef, 'tr A-Z a-z');

print $wtr <<INPUT_TO_TR;
LOWERCASE ME
INPUT_TO_TR
close $wtr;

while (<$rdr>) {
  print "READ​: $_";
}

waitpid $pid, 0;
print "child done​: $?\n";

@p5pRT
Copy link
Author

p5pRT commented Jun 7, 2014

From dmitri@devio.us

This change is for IPC​::Open3 to start working with tied STDERR.

  - Dmitri.

@p5pRT
Copy link
Author

p5pRT commented Jun 7, 2014

From dmitri@devio.us

0001-perl-119843-untie-STDERR-in-IPC-Open3.patch
From 8db9415b02c27cc83ba6d9da2ddee833596a929d Mon Sep 17 00:00:00 2001
From: Dmitri Tikhonov <dmitri@cpan.org>
Date: Sat, 7 Jun 2014 00:23:50 -0400
Subject: [PATCH] [perl #119843] untie STDERR in IPC::Open3

---
 ext/IPC-Open3/lib/IPC/Open3.pm |  1 +
 ext/IPC-Open3/t/IPC-Open3.t    | 29 ++++++++++++++++++++++++++++-
 2 files changed, 29 insertions(+), 1 deletion(-)

diff --git a/ext/IPC-Open3/lib/IPC/Open3.pm b/ext/IPC-Open3/lib/IPC/Open3.pm
index c8620b7..68c1daa 100644
--- a/ext/IPC-Open3/lib/IPC/Open3.pm
+++ b/ext/IPC-Open3/lib/IPC/Open3.pm
@@ -246,6 +246,7 @@ sub _open3 {
 		# A tie in the parent should not be allowed to cause problems.
 		untie *STDIN;
 		untie *STDOUT;
+		untie *STDERR;
 
 		close $stat_r;
 		require Fcntl;
diff --git a/ext/IPC-Open3/t/IPC-Open3.t b/ext/IPC-Open3/t/IPC-Open3.t
index 6ab519d..05aeb4a 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 => 38;
+use Test::More tests => 44;
 
 use IO::Handle;
 use IPC::Open3;
@@ -187,3 +187,30 @@ foreach my $handle (qw (DUMMY STDIN STDOUT STDERR)) {
     }
     waitpid $pid, 0;
 }
+
+# Test that tied STDIN, STDOUT, and STDERR do not cause open3 any discomfort.
+# In particular, tied STDERR used to be able to prevent open3 from working
+# correctly.  RT #119843.
+{
+    {	# This just throws things out
+	package My::Tied::FH;
+	sub TIEHANDLE { bless \my $self }
+	sub PRINT {}
+	# Note the absence of OPEN and FILENO
+    }
+    my $message = "japh\n";
+    foreach my $handle (*STDIN, *STDOUT, *STDERR) {
+	tie $handle, 'My::Tied::FH';
+	my ($in, $out);
+	my $pid = eval {
+	    open3 $in, $out, undef, $perl, '-ne', 'print';
+	};
+	is($@, '', "no errors calling open3 with tied $handle");
+	print $in $message;
+	close $in;
+	my $japh = <$out>;
+	waitpid $pid, 0;
+	is($japh, $message, "read input correctly");
+	untie $handle;
+    }
+}
-- 
1.9.0

@p5pRT
Copy link
Author

p5pRT commented Jun 7, 2014

From @jkeenan

On Fri Jun 06 21​:36​:03 2014, dmitri@​devio.us wrote​:

This change is for IPC​::Open3 to start working with tied STDERR.

- Dmitri.

Thanks for the test file. I have created a smoke testing branch for this at​:

smoke-me/jkeenan/119843-ipc-open3-2

Assuming there are no relevant smoke failures and no objections, I'll merge this to blead in about 7 days.

Thank you very much.
Jim Keenan

@p5pRT
Copy link
Author

p5pRT commented Jun 11, 2014

From @jkeenan

On Sat Jun 07 05​:43​:39 2014, jkeenan wrote​:

On Fri Jun 06 21​:36​:03 2014, dmitri@​devio.us wrote​:

This change is for IPC​::Open3 to start working with tied STDERR.

- Dmitri.

Thanks for the test file. I have created a smoke testing branch for
this at​:

smoke-me/jkeenan/119843-ipc-open3-2

Assuming there are no relevant smoke failures and no objections, I'll
merge this to blead in about 7 days.

Unfortunately one of our smokers has reported a failure in one of IPC​::Open3's test files​: http​://perl.develop-help.com/raw/?id=162567

This makes me reluctant to merge this patch into blead.

The failure was on Win32 (one of George's smokers). Can anyone reproduce/diagnose?

Thank you very much.
Jim Keenan

@p5pRT
Copy link
Author

p5pRT commented Jun 12, 2014

From @ikegami

On Wed, Jun 11, 2014 at 6​:53 PM, James E Keenan via RT <
perlbug-followup@​perl.org> wrote​:

Unfortunately one of our smokers has reported a failure in one of
IPC​::Open3's test files​: http​://perl.develop-help.com/raw/?id=162567

This makes me reluctant to merge this patch into blead.

The failure was on Win32 (one of George's smokers). Can anyone
reproduce/diagnose?

um.... The change is in a branch not taking on Windows.

$^O ne 'os2' &amp;&amp; $^O ne 'MSWin32' && !FORCE_DEBUG_SPAWN;

Will look at the failing tests.

@p5pRT
Copy link
Author

p5pRT commented Jun 12, 2014

From @ikegami

On Wed, Jun 11, 2014 at 8​:34 PM, Eric Brine <ikegami@​adaelis.com> wrote​:

On Wed, Jun 11, 2014 at 6​:53 PM, James E Keenan via RT <
perlbug-followup@​perl.org> wrote​:

Unfortunately one of our smokers has reported a failure in one of
IPC​::Open3's test files​: http​://perl.develop-help.com/raw/?id=162567

This makes me reluctant to merge this patch into blead.

The failure was on Win32 (one of George's smokers). Can anyone
reproduce/diagnose?

um.... The change is in a branch not taking on Windows.

$^O ne 'os2' &amp;&amp; $^O ne 'MSWin32' && !FORCE_DEBUG_SPAWN;

Will look at the failing tests.

Bad test. The test needs to be skipped on Windows since a related problem
exists on Windows and the patch does nothing to solve the problem on
Windows.

@p5pRT
Copy link
Author

p5pRT commented Jun 12, 2014

From @jkeenan

On Wed Jun 11 17​:48​:44 2014, ikegami@​adaelis.com wrote​:

On Wed, Jun 11, 2014 at 8​:34 PM, Eric Brine <ikegami@​adaelis.com> wrote​:

On Wed, Jun 11, 2014 at 6​:53 PM, James E Keenan via RT <
perlbug-followup@​perl.org> wrote​:

Unfortunately one of our smokers has reported a failure in one of
IPC​::Open3's test files​: http​://perl.develop-help.com/raw/?id=162567

This makes me reluctant to merge this patch into blead.

The failure was on Win32 (one of George's smokers). Can anyone
reproduce/diagnose?

um.... The change is in a branch not taking on Windows.

$^O ne 'os2' &amp;&amp; $^O ne 'MSWin32' && !FORCE_DEBUG_SPAWN;

Will look at the failing tests.

Bad test. The test needs to be skipped on Windows since a related problem
exists on Windows and the patch does nothing to solve the problem on
Windows.

ikegami​: Thanks for investigating that. We won't apply the patch, and I'll relinquish ownership of the ticket until the OP or someone else provides an improved one.

Thank you very much.
Jim Keenan

@p5pRT
Copy link
Author

p5pRT commented Jun 13, 2014

From dmitri@devio.us

This change is for IPC​::Open3 to start working with tied STDERR.

Caught by smokers and pointed out by Eric Brine, the tests included
in the original patch do not need to run on Windows, since this code
path is not taken on Windows. This change is to skip the new tests
when IPC​::Open3 spawns instead of forks.

@p5pRT
Copy link
Author

p5pRT commented Jun 13, 2014

From dmitri@devio.us

0002-untie-STDERR-in-IPC-Open3.patch
From b59d720ce1fd09462685637ae00c852b7b517426 Mon Sep 17 00:00:00 2001
From: Dmitri Tikhonov <dmitri@cpan.org>
Date: Thu, 12 Jun 2014 22:48:45 -0400
Subject: [PATCH 2/2] untie STDERR in IPC::Open3
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="------------1.9.0.msysgit.0"

This is a multi-part message in MIME format.
--------------1.9.0.msysgit.0
Content-Type: text/plain; charset=UTF-8; format=fixed
Content-Transfer-Encoding: 8bit

---
 ext/IPC-Open3/lib/IPC/Open3.pm |  1 +
 ext/IPC-Open3/t/IPC-Open3.t    | 32 +++++++++++++++++++++++++++++++-
 2 files changed, 32 insertions(+), 1 deletion(-)


--------------1.9.0.msysgit.0
Content-Type: text/x-patch; name="0002-untie-STDERR-in-IPC-Open3.patch"
Content-Transfer-Encoding: 8bit
Content-Disposition: attachment; filename="0002-untie-STDERR-in-IPC-Open3.patch"

diff --git a/ext/IPC-Open3/lib/IPC/Open3.pm b/ext/IPC-Open3/lib/IPC/Open3.pm
index c8620b7..68c1daa 100644
--- a/ext/IPC-Open3/lib/IPC/Open3.pm
+++ b/ext/IPC-Open3/lib/IPC/Open3.pm
@@ -246,6 +246,7 @@ sub _open3 {
 		# A tie in the parent should not be allowed to cause problems.
 		untie *STDIN;
 		untie *STDOUT;
+		untie *STDERR;
 
 		close $stat_r;
 		require Fcntl;
diff --git a/ext/IPC-Open3/t/IPC-Open3.t b/ext/IPC-Open3/t/IPC-Open3.t
index 6ab519d..a5ef688 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 => 38;
+use Test::More tests => 44;
 
 use IO::Handle;
 use IPC::Open3;
@@ -187,3 +187,33 @@ foreach my $handle (qw (DUMMY STDIN STDOUT STDERR)) {
     }
     waitpid $pid, 0;
 }
+
+# Test that tied STDIN, STDOUT, and STDERR do not cause open3 any discomfort.
+# In particular, tied STDERR used to be able to prevent open3 from working
+# correctly.  RT #119843.
+SKIP: {
+    if (&IPC::Open3::DO_SPAWN) {
+	skip "Calling open3 with tied filehandles does not work here", 6
+    }
+    {	# This just throws things out
+	package My::Tied::FH;
+	sub TIEHANDLE { bless \my $self }
+	sub PRINT {}
+	# Note the absence of OPEN and FILENO
+    }
+    my $message = "japh\n";
+    foreach my $handle (*STDIN, *STDOUT, *STDERR) {
+	tie $handle, 'My::Tied::FH';
+	my ($in, $out);
+	my $pid = eval {
+	    open3 $in, $out, undef, $perl, '-ne', 'print';
+	};
+	is($@, '', "no errors calling open3 with tied $handle");
+	print $in $message;
+	close $in;
+	my $japh = <$out>;
+	waitpid $pid, 0;
+	is($japh, $message, "read input correctly");
+	untie $handle;
+    }
+};

--------------1.9.0.msysgit.0--


@p5pRT
Copy link
Author

p5pRT commented Jun 13, 2014

From @jkeenan

On Thu Jun 12 19​:57​:03 2014, dmitri@​devio.us wrote​:

This change is for IPC​::Open3 to start working with tied STDERR.

Caught by smokers and pointed out by Eric Brine, the tests included
in the original patch do not need to run on Windows, since this code
path is not taken on Windows. This change is to skip the new tests
when IPC​::Open3 spawns instead of forks.

Latest patch is now being smoked in this branch​:
smoke-me/jkeenan/119843-ipc-open3-3

@p5pRT
Copy link
Author

p5pRT commented Jun 13, 2014

From @jkeenan

On Fri Jun 13 03​:46​:18 2014, jkeenan wrote​:

On Thu Jun 12 19​:57​:03 2014, dmitri@​devio.us wrote​:

This change is for IPC​::Open3 to start working with tied STDERR.

Caught by smokers and pointed out by Eric Brine, the tests included
in the original patch do not need to run on Windows, since this code
path is not taken on Windows. This change is to skip the new tests
when IPC​::Open3 spawns instead of forks.

Latest patch is now being smoked in this branch​:
smoke-me/jkeenan/119843-ipc-open3-3

Unfortunately, now we're getting a *different* error on Win32.

See​: http​://perl.develop-help.com/raw/?id=162858

@p5pRT
Copy link
Author

p5pRT commented Jun 14, 2014

From @ikegami

Looks like the process got killed by the watchdog. I can't see any signs of
IPC​::Open3 being used at all by this test.

On Fri, Jun 13, 2014 at 7​:56 PM, James E Keenan via RT <
perlbug-followup@​perl.org> wrote​:

On Fri Jun 13 03​:46​:18 2014, jkeenan wrote​:

On Thu Jun 12 19​:57​:03 2014, dmitri@​devio.us wrote​:

This change is for IPC​::Open3 to start working with tied STDERR.

Caught by smokers and pointed out by Eric Brine, the tests included
in the original patch do not need to run on Windows, since this code
path is not taken on Windows. This change is to skip the new tests
when IPC​::Open3 spawns instead of forks.

Latest patch is now being smoked in this branch​:
smoke-me/jkeenan/119843-ipc-open3-3

Unfortunately, now we're getting a *different* error on Win32.

See​: http​://perl.develop-help.com/raw/?id=162858

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

@p5pRT
Copy link
Author

p5pRT commented Jun 21, 2014

From @greerga

On Wed, 11 Jun 2014, James E Keenan via RT wrote​:

On Sat Jun 07 05​:43​:39 2014, jkeenan wrote​:

On Fri Jun 06 21​:36​:03 2014, dmitri@​devio.us wrote​:

This change is for IPC​::Open3 to start working with tied STDERR.

- Dmitri.

Thanks for the test file. I have created a smoke testing branch for
this at​:

smoke-me/jkeenan/119843-ipc-open3-2

Assuming there are no relevant smoke failures and no objections, I'll
merge this to blead in about 7 days.

Unfortunately one of our smokers has reported a failure in one of IPC​::Open3's test files​: http​://perl.develop-help.com/raw/?id=162567

This makes me reluctant to merge this patch into blead.

The failure was on Win32 (one of George's smokers). Can anyone reproduce/diagnose?

Test output was​:

# Failed test 'no errors calling open3 with tied *main​::STDIN'
# at t/IPC-Open3.t line 208.
# got​: 'open3​: Can't locate object method "FILENO" via package "My​::Tied​::FH" at ../../lib/IPC/Open3.pm line 369.
# '
# expected​: ''
Use of uninitialized value $pid in waitpid at t/IPC-Open3.t line 212.
# Looks like you planned 44 tests but ran 39.
# Looks like you failed 1 test of 39 run.
# Looks like your test exited with 255 just after 39.
../ext/IPC-Open3/t/IPC-Open3.t ....................................
Dubious, test returned 255 (wstat 65280, 0xff00)
Failed 6/44 subtests
  (less 1 skipped subtest​: 37 okay)

--
George Greer

@p5pRT
Copy link
Author

p5pRT commented Jun 21, 2014

From @greerga

On Fri, 13 Jun 2014, James E Keenan via RT wrote​:

On Fri Jun 13 03​:46​:18 2014, jkeenan wrote​:

On Thu Jun 12 19​:57​:03 2014, dmitri@​devio.us wrote​:

This change is for IPC​::Open3 to start working with tied STDERR.

Caught by smokers and pointed out by Eric Brine, the tests included
in the original patch do not need to run on Windows, since this code
path is not taken on Windows. This change is to skip the new tests
when IPC​::Open3 spawns instead of forks.

Latest patch is now being smoked in this branch​:
smoke-me/jkeenan/119843-ipc-open3-3

Unfortunately, now we're getting a *different* error on Win32.

See​: http​://perl.develop-help.com/raw/?id=162858

Nevermind my first reply (a few minutes ago)... made the mistake of not
catching up fully on the thread before replying.

The above error (popen.t) is a long-standing known intermittent error in
Win32 smokes and is not representative of any breakage in the patch
itself.

--
George Greer

@p5pRT
Copy link
Author

p5pRT commented Jun 21, 2014

From @jkeenan

On Fri Jun 20 19​:43​:45 2014, perl@​greerga.m-l.org wrote​:

On Fri, 13 Jun 2014, James E Keenan via RT wrote​:

Latest patch is now being smoked in this branch​:
smoke-me/jkeenan/119843-ipc-open3-3

Unfortunately, now we're getting a *different* error on Win32.

See​: http​://perl.develop-help.com/raw/?id=162858

The above error (popen.t) is a long-standing known intermittent error in
Win32 smokes and is not representative of any breakage in the patch
itself.

George, Thanks for that advice. On that basis, I have applied Dmitri's June 7 patch to blead as commit 04e0f0c.

Marking ticket resolved.

Thanks to all.

@p5pRT
Copy link
Author

p5pRT commented Jun 21, 2014

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

@p5pRT p5pRT closed this as completed Jun 21, 2014
@p5pRT
Copy link
Author

p5pRT commented Jun 21, 2014

From @greerga

On Sat, 21 Jun 2014, James E Keenan via RT wrote​:

On Fri Jun 20 19​:43​:45 2014, perl@​greerga.m-l.org wrote​:

On Fri, 13 Jun 2014, James E Keenan via RT wrote​:

Latest patch is now being smoked in this branch​:
smoke-me/jkeenan/119843-ipc-open3-3

Unfortunately, now we're getting a *different* error on Win32.

See​: http​://perl.develop-help.com/raw/?id=162858

The above error (popen.t) is a long-standing known intermittent error in
Win32 smokes and is not representative of any breakage in the patch
itself.

George, Thanks for that advice. On that basis, I have applied Dmitri's June 7 patch to blead as commit 04e0f0c.

Marking ticket resolved.

Thanks to all.

The patch applied to blead is missing the additional lines from the
smoke-me branch​:

+ if (&IPC​::Open3​::DO_SPAWN) {
+ skip "Calling open3 with tied filehandles does not work here", 6
+ }

--
George Greer

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant