readline: clear the error flag if the error happens to be EAGAIN

[tonycoz]

(or the equivalent EWOULDBLOCK)

This allows questionable code that tries to combine select and the
readline flavour of buffered I/O to limp along. Such code is still
risky due to select() checking the underlying OS handle and not
the perl handle.

Fixes #22883

---

• This set of changes requires a perldelta entry, and it is included.

[toddr]

@tonycoz Thank you for this! Looks like there's a rebase needed to get this merged.

[Leont]

This would mean that suddenly readline returning undef doesn't always mean EOF. That could be quite confusing.

[jkeenan]

@tonycoz @toddr, Could you evaluate @Leont's comment from last month? It's not clear to me that this p.r. should be in 'Approved' status. Thanks.

[tonycoz]

@book, @ap , @haarg I request the guidance of the PSC.

In response to #20060, in #20103 I changed the error handling for readline(), this was applied for perl 5.38 and was documented in perl5380delta.

Previously it would clear the error flag on the stream on pretty much any error, so if there was any sort of error reading from the stream the error flag for the stream would simply be cleared. This was changed to clear the error state in much more limited cases.

This can include errors like trying to read from an output handle, reading from a handle opened to a directory and EWOULDBLOCK errors on sockets marked as non-blocking. This could also include hard errors like EIO

Note that readline() was the only read op that cleared the error state like this, using read() or getc() instead does not clear the stream error state.

This has caused some breakage downstream, including each of the soft error cases above (#22883 #20376 #21240)

In this particular PR after some discussion in #22883 I suggested only ignoring the non-blocking file handle errors, and this PR implements that, though mixing buffered I/O and select() is generally a mistake.

So what I'm looking for here is: should readline() revert to clearing the error state? (and inconsistency with read(), getc())

If not, should the change here be applied, clearing the error state only on EWOULDBLOCK/EAGAIN?

[ap]

We talked this through at the in-person PSC meeting at the PTS today.

So what I'm looking for here is: should readline() revert to clearing the error state? (and inconsistency with read(), getc())

No. That would just resurrect #20060. Fixing that by removing the error-clearing from the end of readline was correct. The previous situation was already conceptually wrong, but that was much less obvious before we addressed #20060.

Namely, the conceptual problem is that, in code which does a whole sequence of I/O operations without checking for errors, and then does an I/O operation after which it does check for errors, that error check can run into an error from any of the preceding I/O operations, even though it’s (99.999% of the time anyway) written to check for errors of only the last operation.

So what we think should be done about #22883 in conjunction with #20060 is that every I/O operation needs to be preceded by clearing the error state. Then readline won’t need to do anything special about EAGAIN and EWOULDBLOCK either, since in those cases it proceeds to do another read internally anyway, prior to which is going to clear the error state, which handles those cases implicitly. (This would automatically keep it correct, for free, even if handling of any new conditions ever were to be added in future – not that that’s likely, just as evidence of conceptual correctness.)

Does that sound right? Are we missing anything @tonycoz?

[tonycoz]

So what we think should be done about #22883 in conjunction with #20060 is that every I/O operation needs to be preceded by clearing the error state. Then readline won’t need to do anything special about EAGAIN and EWOULDBLOCK either, since in those cases it proceeds to do another read internally anyway, prior to which is going to clear the error state, which handles those cases implicitly. (This would automatically keep it correct, for free, even if handling of any new conditions ever were to be added in future – not that that’s likely, just as evidence of conceptual correctness.)

If I understand what you're suggesting, if the user does:

# assuming the output buffer is full or close to it here
print "Hello"; # disk is full, this fails, some output discarded
# some disk space freed here
print "Goodbye"; # error flag cleared, space available, so this succeeds

In particular, if this happened for a perl -pe '....' if the last print succeeded, we'd silently produce a corrupt output file, which is why the error flag is normally preserved.