You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
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:
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:
subclose {
my ($self) = @_;
# close all pipesmy ( $in, $out, $err ) = @{$self}{qw( stdin stdout stderr )};
$inand$in->opened and$in->close || carp "error closing stdin: $!";
$outand$out->opened and$out->close || carp "error closing stdout: $!";
$errand$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?