cpan-authors / IPC-Run

https://metacpan.org/pod/IPC::Run
Other
21 stars 38 forks source link

Spurious warning '"unknown result, unknown PID" isn't numeric ..." has reappeared #169

Open davidlevner opened 1 year ago

davidlevner commented 1 year ago

I'm using IPC::Run::start and

$harness = IPC::Run::start(\@command_words, '<pipe', $stdin_handle, '>pipe', $stdout_handle, '2>pipe', $stderr_handle);
...
$harness->finish();
my $exit_code = $harness->result(0);

Although the program I'm running seems to work properly, I get warnings every time I run it:

Argument "unknown result, unknown PID" isn't numeric in right bitshift (>>) at /usr/share/perl5/vendor_perl/IPC/Run.pm line 3594.

I get the warning twice per run. This seems very similar to CPAN bug 5852 that was apparently resolved. In fact, lines 3619-3620 of Run.pm seem to handle an identical issue in the results method. Perhaps a similar patch can be applied at line 3594 in the result method?

For completeness, I'm using Perl5 (revision 5 version 36 subversion 0) undef Fedora Linux 37. I recently upgraded from Fedora 36 to Fedora 37 and was not seeing these warnings under Fedora 36.

nmisch commented 1 year ago

Perhaps a similar patch can be applied at line 3594 in the result method?

Agreed; probably like this:

diff --git a/lib/IPC/Run.pm b/lib/IPC/Run.pm
index 77863cb..7b7555e 100644
--- a/lib/IPC/Run.pm
+++ b/lib/IPC/Run.pm
@@ -3598,14 +3598,16 @@ sub result {
     &_assert_finished;
     my IPC::Run $self = shift;

+    # we add 0 to stop warnings associated with "unknown result, unknown PID"
     if (@_) {
         my ($which) = @_;
-        return $self->_child_result($which) >> 8;
+        return (0 + $self->_child_result($which)) >> 8;
     }
     else {
         return undef unless @{ $self->{KIDS} };
         for ( @{ $self->{KIDS} } ) {
-            return $_->{RESULT} >> 8 if $_->{RESULT} >> 8;
+            my $candidate = (0 + $_->{RESULT}) >> 8;
+            return $candidate if $candidate;
         }
     }
 }

Could you make a self-contained test case for the warning you were seeing? I'm not seeing it with this quick try:

use IPC::Run 'start';
use strict;
use warnings;

my $h = start
  ['cat'],
  '<pipe',  \*IN,    # may also be a lexical filehandle e.g. \my $infh
  '>pipe',  \*OUT,
  '2>pipe', \*ERR
  or die "cat returned $?";
print IN "some input\n";
close IN;
print <OUT>, <ERR>;
finish $h;
my $x;
$h->result(0);
davidlevner commented 1 year ago

Unfortunately, I am now unable to reproduce the problem. I update my system once a week; maybe some other fix made the problem go away? Anyway, I'm closing this issue because I can't provide a test case that demonstrates the "bug".

nmisch commented 1 year ago

I was able to reproduce it by ignoring SIGCHLD:

use IPC::Run 'start';
use strict;
use warnings;

$SIG{CHLD} = 'IGNORE';
my $h = start
  ['cat'],
  '<pipe',  \*IN,    # may also be a lexical filehandle e.g. \my $infh
  '>pipe',  \*OUT,
  '2>pipe', \*ERR
  or die "cat returned $?";
print IN "some input\n";
close IN;
print <OUT>, <ERR>;
finish $h;
$h->result(0);
$h->results;

The 0+$string approach now gets a warning as well. Could fix it like this:

diff --git a/lib/IPC/Run.pm b/lib/IPC/Run.pm
index 77863cb..9103c5b 100644
--- a/lib/IPC/Run.pm
+++ b/lib/IPC/Run.pm
@@ -3594,18 +3594,25 @@ sub _child_result {
     return $self->{KIDS}->[$which]->{RESULT};
 }

+# stops warnings associated with "unknown result, unknown PID"
+sub _zero_if_non_numeric {
+    no warnings 'numeric';
+    return 0 + shift;
+}
+
 sub result {
     &_assert_finished;
     my IPC::Run $self = shift;

     if (@_) {
         my ($which) = @_;
-        return $self->_child_result($which) >> 8;
+        return _zero_if_non_numeric($self->_child_result($which)) >> 8;
     }
     else {
         return undef unless @{ $self->{KIDS} };
         for ( @{ $self->{KIDS} } ) {
-            return $_->{RESULT} >> 8 if $_->{RESULT} >> 8;
+            my $candidate = _zero_if_non_numeric($_->{RESULT}) >> 8;
+            return $candidate if $candidate;
         }
     }
 }
@@ -3625,8 +3632,7 @@ sub results {
     &_assert_finished;
     my IPC::Run $self = shift;

-    # we add 0 here to stop warnings associated with "unknown result, unknown PID"
-    return map { ( 0 + $_->{RESULT} ) >> 8 } @{ $self->{KIDS} };
+    return map { _zero_if_non_numeric($_->{RESULT}) >> 8 } @{ $self->{KIDS} };
 }
davidlevner commented 1 year ago

I checked my code and it was not setting SIGCHLD. But because the bug can be reproduced and a fix is available (thank you), I'm going to reopen the issue.