cyrusimap / cassandane

Other
6 stars 11 forks source link

run_command() doesn't return exit status if it had to redirect stdin to a scalar #52

Closed wolfsage closed 6 years ago

wolfsage commented 6 years ago

run_command() returns undef if it had a redirected stdin to a scalar. This affects deliver() for example, since deliver() does:

    $self->run_command({
        cyrus => 1,
        redirects => {
            stdin => \$str
        }
    }, @cmd);

run_command() calls _fork_command:

    [...]
    if (defined($redirects{stdin}) && (ref($redirects{stdin}) eq 'SCALAR'))
    {
        my $fh;
        my $data = $redirects{stdin};
        $redirects{stdin} = undef;
        # Use the fork()ing form of open()
        my $pid = open $fh,'|-';
        die "Cannot fork: $!"
            if !defined $pid;
        if ($pid)
        {
            # parent process
            $self->_add_child($binary, $pid, $options->{handlers}, $fh);
            print $fh ${$data};
            close ($fh);
            return $pid;
        }
    }

The close($fh) actually waits for the child to exit and cleans up the process. This means the later waitpid($pid, 0) in reap_command gets a -1:

sub reap_command
{
    my ($self, $pid) = @_;

    # parent process...wait for child
    my $child = waitpid($pid, 0);
    # and deal with it's exit status
    return $self->_handle_wait_status($pid)
        if $child == $pid;
    return undef;
}

So we return undef at the end.

(Relevant pod from perldoc -f perlopen):

            Closing any piped filehandle causes the parent process to wait for
            the child to finish, then returns the status value in $? and
            "${^CHILD_ERROR_NATIVE}".
elliefm commented 6 years ago

Housekeeping, somehow this didn't auto-close when the PR was merged