Open3 and bad gut feeling

[coreolyn]

This sub is at the core of controlling / monitoring all external functionality my Perl scripts will be forced to utilize. It is imperative that I trap and log STDERR which is why I went with Open3. However, Niether of the Camels read clearly to me on bidirectional communication, and just looking at this code I get the feeling I'm setting myself up for headaches long term. It's intended to be handed off to others instead of having to support it myself.

In particular I'm concerned with my assumptions about gathering $stdout and $stderr, and definately have my ears open to all input

# execute() runs a program on any platform and traps
# the STDOUT and STDERR.  A Calling program must handle
# the return value to determine if it worked or not.
# if there is a $@ (trapped error) there are BIG problems

# There is a danger in overruning the Perl buffers
# (if they exceed 255 chars) with either the commandline writer 
# or the output buffer reader this function gets around this by 
# (when necessary) creating a batch file in a temporary directory 
# and then executing that file.

sub execute {
    my $execute = $_[0];
    my $logfile = $_[1];
    my $writer = IO::Handle->new();
    my $reader = IO::Handle->new();
    my $debug = "false";
    
    #$debug = "true";
    if ( $debug =~ /true/i && $logfile ) {
        print "::Utility::execute::\$execute = $execute\n";
    } elsif ( $debug =~ /true/i ) {
            print "::Utility::execute::\$logfile = $logfile\n";
    }
    
    local (*FILE);
    if ( (length $execute) > 254 )     {
        my $workdir = OSworkdir();
        OSmkdir($workdir);
        my $filename;
        if ( $^O =~ /win/i ) {
            $filename = "$workdir"."/execute.bat";
            $filename = OSpath($filename);
            open ( FILE, ">$filename" ) or OSlogger( $logfile, "::Util
+ity::execute Can't open $filename", 1) and die "aw hell";        
        } elsif ( $^O =~ /solaris/i ) {
            $filename = OSpath("$workdir/execute.sh");
            open ( FILE, 774, ">$filename" ) or OSlogger( $logfile, ":
+:Utility::execute Can't open $filename", 1) and die "aw hell";
        }
        print FILE $execute;
        close (FILE);
        OSlogger( $logfile, "Created executable $filename", 1);
        $execute = $filename;
    } 
    if ( $logfile ) {
        OSlogger( $logfile, "Executing $execute", 1);
    }
    
    # Here's the actual execution.
    my $pid;
    eval { $pid = open3( $writer, $reader, $reader, $execute ); };
    die "::Utility::execute died a horrible death!\n" if $@;
    close($writer);
    
    # Lets get STDOUT and STDERR ( note the first read always(?) seems
+ to get nothing)
    my $stdout = <$reader> ;
    $stdout = <$reader> ;
    my $return_val = $stdout;
    my @stderr;
    while (<$reader>) {
        push (@stderr, $_ );
    }
    if ( $logfile ) {
        OSlogger( $logfile,"\$run RETURNED:  $stdout\n", 1);
    }

    if ( @stderr ) {
        OSlogger( $logfile,"Error on $execute\n\n  @stderr\n", 1);
        $return_val = @stderr;
    }
    
    # Return the output
    return $return_val;
}

coreolyn

Edit Masem 2002-02-19 - Added READMORE tag after 1st para

[particle]

i've used IPC::Open3 because i needed to capture STDIN, STDERR, return code, and pid at once, and it's the only way to do it. you can find a summary on my implementation at once again: program output and return code

this code is running on a production system that synchronizes code between PVCS repositories. it's been running for over a year, and there's only ever been one problem: sporadic failures with IO::Pipe. see my question here: intermittent problem with IPC::Open3. finding no other resolution, i used a short sleep command.

perhaps you'll get some mileage from this.

~Particle

[coreolyn]

Thanks particle. Great node. It looks like you were going over the same nodes I was. Finishing that node really helps a lot. I think I can make a few changes and have it much more legible about what the hell is going on. I'm a bit worried about the resource error (maxed filehandles?). I guess should set a trap for that one right off the bat. How long did you sleep before the retry?

coreolyn

[particle]

sleep(1); ought to do it. it's quite sporatic (~1 per 1000 calls) and frustrating.

the production server is a 180MHz pentiumpro w/256MB RAM, running NT 4 SP6, but it has ultra-fast-wide scsi, and a fat network pipe. i did not see this problem on my (then) desktop, a dual pIII 450. your mileage may vary.

good luck!

~Particle

[tilly]

A sneaking feeling tells me that you will shortly be Suffering from Buffering. (There is a similar warning in IPC::Open3.) Namely if there is, for instance, a long error statement, you may be trying to read from its STDOUT while it is waiting for you to check its STDERR. This might take a while...

You are likely to need the select command or its OO wrapper IO::Select to do this properly. (I think you will find IO::Select easier to figure out.)

[coreolyn]

I should mention that CPAN mods are not a viable option around here (That doesn't reflect my policy just my employers). coreolyn

[mojotoad]

Why aren't CPAN modules a viable option? Is this because they are forbidden by policy or because you do not have control over the perl installation tree?

In the latter case, at least, you could install CPAN modules in a lib directory distributed with your application. FindBin and use lib are your friends after that (see FindBin for examples).

Matt

[coreolyn]

While policy does not explicitly forbid CPAN modules, they are completely unsupported inhouse. Without inhouse support there's too many hurdles to overcome to use CPAN modules effectively. It's more like enforcing a ban by attrition than an outright ban.

I'm working on 'em though projects like the one I'm working on currently will go a long way to insinuate Perl a little deeper into the corporate culture/infra-structure

For now that has to be the best next step. Once they are comfortable with that then progress on CPAN can be made. I know that might sound a bit backword to outsiders but I've been advocating actively for a few years here. Patience and solid code in the clutch is the best course for now :)

coreolyn

[particle]

coreolyn, i've been following this node very closely. you can see by my replies that i have been trying to solve a similar issue. i was never satisfied with my implementation... mostly due to tilly's constant, nagging reminders to use select :)

well, i did a little investigation on select, and i found some interesting things you'll appreciate. for instance, select is only implemented on sockets in Win32. hrmm. also, open2/open3 cannot redirect input/output in a child process via fork in Win32. i'm afraid a lot of the code you have there is not doing anything useful.

here's a list of links for more information:

perlport
IO::Select on Windows
stdin-socket without fork()??


here's a test script to verify select is in fact, not working. can_write, can_read, and has_exception all come back undef on Win32... (see below)

#!/usr/bin/perl
use IO::Select;
# create typeglobs and add to selector
$output_select = IO::Select->new( *CMD_ERR, *CMD_OUT, *CMD_IN );
# test selector methods
$cnt = $output_select->count();
print "count:     $cnt\n";
@bob = $output_select->handles();
print "list:      @bob\n";
@bob = $output_select->can_write(10);
print "write:     @bob\n";
@bob = $output_select->can_read(10);
print "read:      @bob\n";
@bob = $output_select->has_exception(10);
print "exception: @bob\n";
close(CMD_IN);
close(CMD_OUT);
close(CMD_ERR);

i'm now working on a client / server solution for executing external programs and redirecting output using sockets. as soon as i have something solid, i'll be sure to post it, and /msg you with the node_id.

thank you for using microsoft ;-D

~Particle

[Random_Walk]

Hi particle

I have been working on a similar problem Win32 capturing output from a process that may hang, capturing stdout and stderr from a child proc, must work under windows and unix and there is a good chance the child will hang and have to be killed.

I have a partial solution at Re^4: Win32 capturing output from a process that may hang using threads and non blocking thread queues. This does not however do stderr properly. I have started trying to use open3 for this but have found it a can of worms. Did you get anywhere with the client server socket talking approach ?

Cheers,
R.

[particle]

actually, i later found IPC::Run, which does a great job for me, even though it says win32 support is *experimental*. in fact, i'm using happily using it right now to glue two apps together :)

[coreolyn]

Arg..

I had thought I had overcome the problem of executing large command lines by writing the command line into script that is executed. However, even though the created script runs fine directly from the command line, I have found that if I run it through the attached sub and the line to be executed execeeds 357 characters I 'hang' (MSwin32 - 2000). Additionally I have attempted to implement IO::Handle and IO::Select but apparently not in an effective way. (If it is indeed some type of a buffering problem). I lost all day yesterday to this code and hope someone can pinpoint my ignorance, or am I just exceeding the limitations of open3?

Here's the relevant code. (note: I pushed all the logging to a separate module)

# RETURNS: %OSexec = ( stdout => $stdout, stderr => $stderr pid => $pi
+d )
sub OSexecute {
    my $execute = $_[0];
    my $stdin = IO::Select->new();
    # not sure what difference <&STDIN and \*STDIN have
    $stdin->add("\*STDIN");
    my $din = IO::Handle->new();
    $stdin->add($din);
         
    my $stdout = IO::Select->new();
    $stdout->add(\*STDOUT);
    my $dout = IO::Handle->new();
    $stdout->add($dout);
         
    my $stderr = IO::Select->new();
    $stderr->add(\*STDERR);
    my $derr = IO::Handle->new();
    $stderr->add($derr);
         
    my ( $pid, $val, $out, $err );
    my ( @stdin, @stdout, @stderr );
    my $debug = "false";

    #$debug = "true";
    if ( $debug =~ /true/i ) {
        print "OSify::Execute::OSexecute::\$execute = $execute\n";
    }
        
    # Here is the actual execution
    eval {
        print "Executing $execute\n";
           $pid = open3( $din, $dout, $derr, $execute );
           print "Waiting for pidout\n";
        # waitpid waits for the proces to exit
        # $val could be used as a means to determine status
        # while waiting if that functionality becomes needed.
           $val = waitpid(-1,0); #wait's for process to complete
                 
        # Process the results
        # Standard Out
        my $line;
        my @stdout = <$dout>;
        foreach $line (@stdout) {
            chomp($line);
             $out = $out . $line;
        }
        if ( ! $out ) {
            $out = 1;
        }
         
        # Standard Error
         @stderr = <$derr>;
        foreach $line ( @stderr ) {
            chomp($line);
             $err = $err . $line;
        }

        if ( ! $err ) {
            $err = 1;
        }
    };
    $@ && die "OSify::OSexecute died upon execution of\n$execute\nWith
+$@";

    # Question remains what activity qualifies as draining the buffer?

    $din->flush();
    $din->close;
    $dout->flush();
    $dout->close;
    $derr->flush();
    $derr->close;
    
    my %OSexec = ( 
        stdout => $out, 
        stderr => $err,
        pid => $pid, 
    );
    print "Execute Finished with @{[%OSexec]}\n";
    return %OSexec;
}

Here's a very typical script it will execute. The antcall.bat doesn't even have to exist to duplicate the problem.

E:\\cccharv\\JDK\\Release\\scripts\\antcall.bat -logfile E:\\cccharv\\
+testApp\\testVer\\Unit_Test\\antscripts\\build\\logs\\20020219-082532
+_Ant_build_testApp.txt -buildfile E:\\cccharv\\testApp\\testVer\\Unit
+_Test\\antscripts\\buildTESTAPP.xml compileTESTAPPClean buildTESTAPPJ
+ar buildTESTAPPWar buildClientControllerEJB buildTESTAPPSessionEJB bu
+ildTESTAPPEar

Getting rid of the last three characters eliminates the hang

coreolyn

[coreolyn]

Well I've narrowed the problem to STDOUT. If I turn @echo off I've eliminated the problem. Which for this particular app is ok, but I'd sure like to see the problem eliminated completely and to get a better handle on filehandles/buffering.

Update:Aw frick'n frack'n bull riffin DOS frmbln grrr..

@echo off just fixed the Perl problem now the OS can't handle it... This is why I quit developing on DOS!!!!

coreolyn ... mumble mumble mumble.

[coreolyn]

Welp.. turns out that if I pass the command+args as an array instead of a string to Open3 the problem disapears. I don't even have to create an script to run large command lines.

The DOS problem was eliminated by slurping up the args via set VAR=%*.

So to put an end to this here's the code as I am going to run with it.

# RETURNS: %OSexec = ( stdout => $stdout, stderr => $stderr pid => $pi
+d )
sub OSexecute {
    my @execute = @_;
    my $stdin = IO::Select->new();
    # not sure what difference <&STDIN and \*STDIN have
    $stdin->add("\*STDIN");
    my $din = IO::Handle->new();
    $din->autoflush(1);
    $stdin->add($din);
     
    my $stdout = IO::Select->new();
    $stdout->add(\*STDOUT);
    my $dout = IO::Handle->new();
    $dout->autoflush(1);
    $stdout->add($dout);
         
    my $stderr = IO::Select->new();
    $stderr->add(\*STDERR);
    my $derr = IO::Handle->new();
    $derr->autoflush(1);
    $stderr->add($derr);
         
    my ( $pid, $val);
    my $debug = "false";

    #$debug = "true";
    if ( $debug =~ /true/i ) {
        print "OSify::Execute::OSexecute::\$execute = @execute\n";
    }

    # Here is the actual execution
    eval {
           $pid = open3( $din, $dout, $derr, @execute );
        # waitpid waits for the proces to exit
        # $val could be used as a means to determine status
        # while waiting if that functionality becomes needed.
           $val = waitpid(-1,0); #wait's for process to complete
    };                 
    $@ && die "OSify::OSexecute died upon execution of\n@execute\nWith
+$@";

    # Gather the results
    my $line;
    # Standard Out
    
    my @stdout = <$dout>;
    my $out;
    foreach $line (@stdout) {
        $line = OSify::Utility::trimSpaces($line);
        $out = $out . $line;
    }
    if ( ! $out ) {
        $out = 1;
    }
         
    # Standard Error
     my @stderr = <$derr>;
     my $err;
    foreach $line ( @stderr ) {
        $line = OSify::Utility::trimSpaces($line);
         $err = $err . $line;
    }

    if ( ! $err ) {
        $err = 1;
    }

    # Flush and close the Filehandles
    $din->flush();
    $din->close;
    $dout->flush();
    $dout->close;
    $derr->flush();
    $derr->close;
    
    my %OSexec = ( 
        stdout => $out, 
        stderr => $err,
        pid => $pid, 
    );
    
    return %OSexec;
}

coreolyn

[ikegami]

This code can be trimmed down a lot.

• $stdin, $stdout and $stderr are never used, so they should be deleted.
• Flushing does nothing for input handles, so $dout->autoflush(1);, $derr->autoflush(1);, $dout->flush(); and $derr->flush(); do nothing.
• $din->flush(); is useless for two reason: 1) The program on the other side of the pipe has ended, and 2) the close does a flush.
• $din->autoflush(1); is not needed since we don't write to $din. Even if we did, open3 makes it autoflush for us.
• Since we don't do anything with $din, $dout and $derr anymore, we can let open3 create them for us.
• Filehandles are automatically closed when they go out of scope.
• The $pid is for a dead process, so why return it to the caller?
• It's more common to use 0/1 for boolean instead of "false"/"true". Deviations make it less readable, and strings make it more complicated. Using a regular expression to check the equality of two strings is overkill. eq is the code for that.
• Reformatting $out and $err has nothing to do with OSexecute and should be done by the calling function if it so desires.

There are many small problems.

• $stdin->add("\*STDIN"); should be $stdin->add(\*STDIN);.
• $val = waitpid(-1, 0); should be $val = waitpid($pid, 0);. $val isn't used, so the assignment can be ommited as well.
• "@execute" will cram all the arguments into one word.
• Why change a stdout and stderr to 1 if it's empty, when it's just as easy to check for the empty string ($str eq '' and length($str)
• $stdout is changed from to 1 if it's '0'. Same for $stderr.
• Returning a hash is... odd. If the code did return ($stdout, $stderr);, the caller could very simply do my ($stdout, $stderr) = OSexecute(...);

Cleaned Up Code

# RETURNS: ( $stdout, $stderr )  Both are refs to an array of lines.
sub OSexecute {
    my @execute = @_;

    local $, = ' ';  # for print "...@execute..."

    my $debug = 0;
    #$debug = 1;
    print("OSexecute(@execute)\n") if ($debug);

    # Here is the actual execution.
    my $pid = eval { open3($din, $dout, $derr, @execute) };
    die "OSexecute(@execute): $@" if ($@);

    # Wait for process to complete.
    waitpid($pid, 0);

    # Gather the results
    my @stdout = <$dout>;
    my @stderr = <$derr>;

    # We should check the return code of the child.
    # Gotta trap SIGPIPE for that.

    return ( \@stdout, \@stderr );
}

There's a big problem

{
   my $file_name;

   foreach $file_name ('c:\\tinyfile.txt', 'c:\\biggfile.txt') {
      my ($stdout, $stderr);

      print("$file_name\n", ("=" x length($file_name))."\n", "\n");
      ($stdout, $stderr)
         = OSexecute('cmd.exe', '/c', 'type', "\"$file_name\"");
      print("stdout\n", "------\n", @$stdout);
      print("Nothing was sent to STDOUT.\n") unless (@$stdout);
      print("\n");
      print("stderr\n", "------\n", @$stderr);
      print("Nothing was sent to STDERR.\n") unless (@$stderr);
      print("\n", "\n");
   }
}

outputs

c:\tinyfile.txt
===============

stdout
------
foo
bar
bla

stderr
------
Nothing was sent to STDERR.


c:\biggfile.txt
===============
*** HANGS ***

The problem is that file handles (including $dout and $derr) have a buffer that's limited in size. biggfile.txt is less than 2KB, which is really quite small, so this needs to be fixed.

Fix

# RETURNS: ( $stdout, $stderr )  Both are refs to an array of lines.
sub OSexecute {
    my @execute = @_;

    my @stdout;
    my @stderr;

    local $, = ' ';  # for print "...@execute..."

    my $debug = 0;
    #$debug = 1;
    print("OSexecute(@execute)\n") if ($debug);

    # Here is the actual execution.
    my $pid = eval { open3($din, $dout, $derr, @execute) };
    die "OSexecute(@execute): $@" if ($@);

    my $select = IO::Select->new();
    $select->add($dout);
    $select->add($derr);

    my @ready;
    my $fh;

    # Gather the results
    while (@ready = $select->can_read()) {
       foreach $fh (@ready) {
          push(@stdout, <$fh>) if ($fh == $dout);
          push(@stderr, <$fh>) if ($fh == $derr);
       }
    }

    # Wait for process to complete and reap it.
    waitpid($pid, 0);

    # We should check the return code of the child.
    # Gotta trap SIGPIPE for that.

    return ( \@stdout, \@stderr );
}

Fixed?

Oops! select() (and IO::Select) doesn't work in Windows. That sucks. It means we're gonna have to use threads!!! I have no experience with threads, so maybe another day.