I got IPC::Open3 to work!

After dinking around with an expensive three or four days trying to understand IPC::Open3 and trying to make things work I finally got it to work. I thought I would add my 2 cents and hopefully save someone else (or myself somewhere down the road) the same headache.

Yes, I know about IPC::Run. Unfortunately, the host I was required to use did not have it installed and I was not allowed to install any but my own stuff (I could have claimed it as mine for the purpose of getting things done, but I still didn't understand what exactly I was doing and didn't want to be required to fix something I didn't understand when it broke).

First, the samples given here and elsewhere, while very helpful, included some extraneous details that contributed to my confusion.

Given the *simplest* requirement:

Call a program using command line arguments and passing data via STDIN, grab both STDOUT and STDERR and process them somehow.

All of the examples included code like

my $IN = IO::File->new_tmpfile;
[download]

local *OUT;
open OUT, ">output.log";
[download]

and then proceed to call the open3 with those filehandles.

They aren't needed, at least for something this simple. Once I realized this I was able to get things working:

sub callexternalpgm {
  my ( $cmd, $cmdargs, $rawdata, $processeddata, $err ) = @_;

  my ( $IN, $OUT, $ERR );

  $ERR = gensym();

  my $pid;
  eval{ $pid = open3( $IN, $OUT, $ERR, $cmd, @{ $cmdargs } ) };
  return $@ if $@;

  print $IN $rawdata;
  close $IN;

  my $select = new IO::Select;
  $select->add( $OUT, $ERR );

  while(my @ready = $select->can_read) {
    foreach my $fh (@ready) {
      my $data;
      my $length = sysread $fh, $data, 4096;

      if( ! defined $length || $length == 0 ) {
        $err .= "Error from child: $!\n" unless defined $length;
        $select->remove($fh);
      } else {
        if($fh == $OUT) {
           $processeddata .= $data;
        } elsif($fh == $ERR) {
          $err .= $data;
        } else {
          return undef;
        }
      }
    }
  }

  waitpid $pid, 0; # wait for it to die

  warn $err if $err;
  return 1;
}
[download]

Any comments or improvements would be appreciated. I realize I need to add some checks for references and the like.

Harley J Pig

Comment on I got IPC::Open3 to work! Select or Download Code

Replies are listed 'Best First'.
Re: I got IPC::Open3 to work! by siracusa (Friar) on Jul 23, 2005 at 00:57 UTC
It doesn't look like you're handling ~~partial reads or~~ other exceptional conditions in your call to sysread(). Something like this might be a start. `use POSIX qw(EINTR EWOULDBLOCK EAGAIN); ... # Try to read 4096 bytes from $fh using sysread(). Icky. my $data; my $offset = 0; my $length = 4096; while($length > 0) # Handle partial reads { my $read = sysread($fh, $data, $length, $offset); unless(defined $read) { next if($! == EINTR \|\| $! == EWOULDBLOCK \|\| !$ == EAGAIN); # Handle other errors here ... } last if($read == 0); # EOF $offset += $read; $length -= $read; }` [download] In practice, the details are somewhat reliant on the exact nature of the read(2) system call on your OS. The code above almost certainly does NOT handle all potential errors, but it's a start. The big point is that you can't expect sysread() to actually read as much data as you ask it to read. At the very least, you have to handle partial reads and then either loop until you've read what you want (as shown above) or make some other sort of decision. What you definitely don't want to do is treat something like an undef return caused by EINTR as a fatal error. That type of thing tends to crop up at the worst times (e.g., on a busy machine but never on your idle development box). sysread() has similar annoyances associated with it. If it's at all possible to use the blocking versions (read() and write()) it will make your life a lot easier.	[reply] [d/l]
Re^2: I got IPC::Open3 to work! by ikegami (Patriarch) on Jul 23, 2005 at 07:31 UTC
It doesn't look like you're handling partial reads He does. Relevant code: `while(my @ready = $select->can_read) { ... my $data; my $length = sysread $fh, $data, 4096; ... $processeddata .= $data; ... }` [download] All the data ends up in processeddata, no matter how much is read by a given call to sysread. It doesn't look like you're handling ... other exceptional conditions He does this too. Relevant code: `if( ! defined $length \|\| $length == 0 ) { $err .= "Error from child: $!\n" unless defined $length; $select->remove($fh); }` [download] The filehandle is removed from the list of handles monitored by `select`. I'm guessing `can_read` will return false when it no longer monitors any handles. Something like this might be a start. That's bad! You removed `select`, which is cruicial here.	[reply] [d/l] [select]
Re^3: I got IPC::Open3 to work! by siracusa (Friar) on Jul 23, 2005 at 12:36 UTC
All the data ends up in processeddata, no matter how much is read by a given call to sysread. Oops, my bad. Actually, hm, I'm not sure what happens without the offset adjustments that I have in my code... It doesn't look like you're handling ... other exceptional conditions He does this too. Relevant code: `if( ! defined $length \|\| $length == 0 ) { $err .= "Error from child: $!\n" unless defined $length; $select->remove($fh); }` [download] That treats things like EINTR as "fatal" errors (stop listening on that $fh), which is probably not what he wants. You removed select, which is cruicial here. I was just commenting on the vagaries of sysread().	[reply] [d/l]
Re^4: I got IPC::Open3 to work! by harleypig (Monk) on Jul 24, 2005 at 16:04 UTC
Re: I got IPC::Open3 to work! by ikegami (Patriarch) on Jul 23, 2005 at 07:46 UTC
1) The docs say "open3() does not wait for and reap the child process after it exits." You need to call `waitpid` on children that have completed. 2) Your return value are not the best. You return true when when `open3` fails. You also return true on success. 3) I find it odd that you return `undef` when `can_read` returns a file handle you didn't give it. It shouldn't happen, so I would just ignore the error, since it shouldn't happen anyway. The safest way to handle it would be to do something like: `$err .= "Error from select: can_ready returned bad handle\n"; $select->remove($fh);` [download]	[reply] [d/l] [select]
Re^2: I got IPC::Open3 to work! by Tanktalus (Canon) on Jul 23, 2005 at 13:37 UTC
Re #3 - I'm not sure I follow. If it shouldn't happen, it shouldn't matter how it's handled. Besides, I'd rather be woken up in the middle of the night by something that shouldn't happen but did, rather than mask the error by ignoring it. My house shouldn't catch fire, for example... ;-)	[reply]
Re^3: I got IPC::Open3 to work! by ikegami (Patriarch) on Jul 23, 2005 at 16:35 UTC
Besides, I'd rather be woken up in the middle of the night by something that shouldn't happen but did, rather than mask the error by ignoring it. The `die`ing would be a better alternative then returning `undef`, which simply displaces the error to some other indeterminate location.	[reply] [d/l] [select]
Re^2: I got IPC::Open3 to work! by harleypig (Monk) on Jul 24, 2005 at 16:01 UTC
The function only returns undef in that instance. I hate having some generic function die on me unexpectedly, especially when my boss calls me up shouting "You're stoopid program doesn't work!" I'd rather leave the choice to the calling program: `callexternalpgm( parms ) or die "$!";` [download] or, more likely, `callexternalpgm( parms ) or LogIt( "Error calling external program: $! +" );` [download] Harley J Pig	[reply] [d/l] [select]
Re: I got IPC::Open3 to work! by zentara (Archbishop) on Jul 23, 2005 at 12:19 UTC
Since you have your mind "wrapped around" IPC::Open3, you might find this interesting. IPC3 buffer limit problem It shows how to use some low level pipe reads. I'm not really a human, but I play one on earth. flash japh	[reply]
Re^2: I got IPC::Open3 to work! by harleypig (Monk) on Jul 28, 2005 at 16:53 UTC
Thank you. That was some very informative reading. Harley J Pig	[reply]


"be consistent"
	PerlMonks