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
Comments
From ailin@devio.usThis is a bug report for perl from ailin@devio.us, I am trying to use IPC::Open3 to run a program from within a FCGI I have confirmed that the issue is still in the Perl 5.19.3 IPC::Open3 untie *STDERR; to the same place as untie *STDOUT; Thanks, Flags: |
From @jkeenanOn Mon Sep 16 01:45:59 2013, ailin@devio.us wrote:
Is the attached the change you are recommending? Thank you very much. |
The RT System itself - Status changed from 'new' to 'open' |
From ailin@devio.usOn Mon, 2013-09-16 at 19:03 -0700, James E Keenan via RT wrote:
Thanks for putting my meaning into a diff file, this is exactly what I |
From @jkeenanOn Wed Sep 18 21:48:00 2013, ailin@devio.us wrote:
Okay, while I've written the patch, I take no position on its merits, as Thank you very much. |
From @ikegamiOn Thu, Sep 19, 2013 at 9:48 PM, James E Keenan via RT <
C<< croak >> can be called after the point where you added C<< untie However, since the time when C<< untie *STDERR; >> was omitted because that As such, adding C<< untie *STDERR; >> looks safe to me. |
From @jkeenanOn Wed Sep 18 21:48:00 2013, ailin@devio.us wrote:
Can you write a unit test for this? I am reluctant to take the "let's apply it to blead and see what breaks Thank you very much. |
From @jkeenanOn Sun Sep 22 10:10:25 2013, jkeenan wrote:
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. |
From @jkeenanOn Mon Nov 18 16:34:18 2013, jkeenan wrote:
Marking ticket stalled. |
@jkeenan - Status changed from 'open' to 'stalled' |
From ailin@devio.usOn Mon, 2013-11-18 at 16:34 -0800, James E Keenan via RT wrote:
Hi, I'm having trouble coming up with a unit test, as I am not an expert in server.document-root = "/var/www" and here is the test script; it dies spectacularly when trying to open3 #!/usr/bin/perl my $count = 0; while ($request->Accept >= 0) { waitpid( $pid, 0 ); You have to open http://localhost:8082/xxx in your browser to run the Here is a stack trace as far as I could produce them: <white screen of death in browser> Modifying IPC::Open3 to untie STDERR fixes this problem consistently, so Care, |
The RT System itself - Status changed from 'stalled' to 'open' |
From dtikhonov@yahoo.comOn Thu Sep 19 21:18:51 2013, ikegami@adaelis.com wrote:
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. |
From dtikhonov@yahoo.com# Test to illustrate RT#119843 in IPC::Open3 use strict; use IPC::Open3 qw(open3); { tie *STDERR, 'My::Tied::FH'; my $pid = open3(my $wtr, my $rdr, undef, 'tr A-Z a-z'); print $wtr <<INPUT_TO_TR; while (<$rdr>) { waitpid $pid, 0; |
From dmitri@devio.usThis change is for IPC::Open3 to start working with tied STDERR. - Dmitri. |
From dmitri@devio.us0001-perl-119843-untie-STDERR-in-IPC-Open3.patchFrom 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
|
From @jkeenanOn Fri Jun 06 21:36:03 2014, dmitri@devio.us wrote:
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. |
From @jkeenanOn Sat Jun 07 05:43:39 2014, jkeenan 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? Thank you very much. |
From @ikegamiOn Wed, Jun 11, 2014 at 6:53 PM, James E Keenan via RT <
um.... The change is in a branch not taking on Windows. Will look at the failing tests. |
From @ikegamiOn Wed, Jun 11, 2014 at 8:34 PM, Eric Brine <ikegami@adaelis.com> wrote:
Bad test. The test needs to be skipped on Windows since a related problem |
From @jkeenanOn Wed Jun 11 17:48:44 2014, ikegami@adaelis.com wrote:
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. |
From dmitri@devio.usThis change is for IPC::Open3 to start working with tied STDERR. Caught by smokers and pointed out by Eric Brine, the tests included |
From dmitri@devio.us0002-untie-STDERR-in-IPC-Open3.patchFrom 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--
|
From @jkeenanOn Thu Jun 12 19:57:03 2014, dmitri@devio.us wrote:
Latest patch is now being smoked in this branch: |
From @jkeenanOn Fri Jun 13 03:46:18 2014, jkeenan wrote:
Unfortunately, now we're getting a *different* error on Win32. See: http://perl.develop-help.com/raw/?id=162858 |
From @ikegamiLooks like the process got killed by the watchdog. I can't see any signs of On Fri, Jun 13, 2014 at 7:56 PM, James E Keenan via RT <
|
From @greergaOn Wed, 11 Jun 2014, James E Keenan via RT wrote:
Test output was: # Failed test 'no errors calling open3 with tied *main::STDIN' -- |
From @greergaOn Fri, 13 Jun 2014, James E Keenan via RT wrote:
Nevermind my first reply (a few minutes ago)... made the mistake of not The above error (popen.t) is a long-standing known intermittent error in -- |
From @jkeenanOn Fri Jun 20 19:43:45 2014, perl@greerga.m-l.org wrote:
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. |
@jkeenan - Status changed from 'open' to 'resolved' |
From @greergaOn Sat, 21 Jun 2014, James E Keenan via RT wrote:
The patch applied to blead is missing the additional lines from the + if (&IPC::Open3::DO_SPAWN) { -- |
Migrated from rt.perl.org#119843 (status was 'resolved')
Searchable as RT119843$
The text was updated successfully, but these errors were encountered: