Skip to content

close fails with irrelevant errors after perl 5.38 #22883

Open
@toddr

Description

After upgrade from perl 5.36 to 5.40, we began encountering issues with code that used Git::Repository. This module uses System::Command to interface with the git command line binary. We implemented a modified $git->run, which does the following:

my $select = IO::Select->new;
foreach my $fh ( $stdout, $stderr ) {
    $fh->blocking(0);
    $select->add($fh);
}

while ( $select->count ) {
    my @ready = $select->can_read;
    foreach my $fh (@ready) {
        if ( eof $fh ) {
            $select->remove($fh);
        }
        else {
            ${ $data->{$fh} } .= $_ while (<$fh>);
        }
    }
}

This code is pretty much straight out of the documentation example in IO::Select.

During cleanup, System::Command does the following to its handles for $stdin/$stdout/$stderr and this is causing a lot of noise right now for us:

sub close {
    my ($self) = @_;

    # close all pipes
    my ( $in, $out, $err ) = @{$self}{qw( stdin stdout stderr )};
    $in  and $in->opened  and $in->close  || carp "error closing stdin: $!";
    $out and $out->opened and $out->close || carp "error closing stdout: $!";
    $err and $err->opened and $err->close || carp "error closing stderr: $!";

    # and wait for the child (if any)
    $self->_reap();

    return $self;
}

It's not uncommon to see this on CPAN, and it's even worse: We see close $fh or die..., which means all of this code now unexpectedly blows up.

Digging deeper into our problem, we determined that the IO::Select method can_read is causing EAGAIN (or is it EWOULDBLOCK? They're the same error) to be put as an error on the file handle, which now causes close to exit non-zero.

In the change introduced by #20103, which addressed issues raised by #20060, they were concerned with losing errors on the file handle during close. While I can see some value in EISDIR being retained, I struggle to understand why we would retain ALL of the errors, especially if there is a successful read afterwards.

I would prefer #20103 to be reverted, but I think we're past that, so I want to suggest an alternative to @tonycoz, who worked on this originally: Possibly, we could restore the PerlIO_clearerr unless the error set are specific ones we care about. In my specific case, I would argue that EAGAIN should be cleared if there is later a successful read, right?

Metadata

Assignees

No one assigned

    Labels

    dist-IOissues in the dual-life blead-first IO distributiontype-core

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions