Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

IO::Handle::error() can lose read errors #20060

Closed
ntyni opened this issue Aug 7, 2022 · 6 comments · Fixed by #20103
Closed

IO::Handle::error() can lose read errors #20060

ntyni opened this issue Aug 7, 2022 · 6 comments · Fixed by #20103
Labels
dist-IO issues in the dual-life blead-first IO distribution

Comments

@ntyni
Copy link
Contributor

ntyni commented Aug 7, 2022

As reported by Ian Jackson in https://bugs.debian.org/1016369 IO::Handle::error() can report 0 when there was an error on the filehandle.

A test case is

perl -MIO::Handle -e 'open X, "<", "." or die $!; $_ = <X>; printf "%s %s\n", X->error(), $!;'

which returns "0 Is a directory" for me on Linux although all reads on the filehandle fail with EISDIR.

I've debugged this a bit and it looks like the error gets cleared in Perl_do_readline() around pp_hot.c:3295 or so with a PerlIO_clearerr() call. I think this is related to reading past EOF or something like that?

While trying to treat a directory as a regular file is something of a corner case (and the result is rather platform dependent), Ian argues that this is an indication of a more general problem where EIO and similar errors can go unnoticed if user code is relying on the IO::Handle::error() method to report them. Of course, creating simple test cases for such errors is difficult if not impossible.

I see #12782 is very similar but hasn't really received much attention. Is this something that should at least be documented as a caveat if it's not fixable?

-----------------------------------------------------------------
---
Flags:
    category=library
    severity=medium
    module=IO::Handle
---
Site configuration information for perl 5.37.3:

Configured by ntyni at Sat Aug  6 17:54:25 BST 2022.

Summary of my perl5 (revision 5 version 37 subversion 3) configuration:
  Commit id: 12d173c94b050c244dbb4e2162e29834a9ac3aff
  Platform:
    osname=linux
    osvers=5.17.0-1-amd64
    archname=x86_64-linux
    uname='linux carme 5.17.0-1-amd64 #1 smp preempt debian 5.17.3-1 (2022-04-18) x86_64 gnulinux '
    config_args='-des -Dusedevel'
    hint=recommended
    useposix=true
    d_sigaction=define
    useithreads=undef
    usemultiplicity=undef
    use64bitint=define
    use64bitall=define
    uselongdouble=undef
    usemymalloc=n
    default_inc_excludes_dot=define
  Compiler:
    cc='cc'
    ccflags ='-fwrapv -fno-strict-aliasing -pipe -fstack-protector-strong -I/usr/local/include -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64 -D_FORTIFY_SOURCE=2'
    optimize='-O2'
    cppflags='-fwrapv -fno-strict-aliasing -pipe -fstack-protector-strong -I/usr/local/include'
    ccversion=''
    gccversion='11.2.0'
    gccosandvers=''
    intsize=4
    longsize=8
    ptrsize=8
    doublesize=8
    byteorder=12345678
    doublekind=3
    d_longlong=define
    longlongsize=8
    d_longdbl=define
    longdblsize=16
    longdblkind=3
    ivtype='long'
    ivsize=8
    nvtype='double'
    nvsize=8
    Off_t='off_t'
    lseeksize=8
    alignbytes=8
    prototype=define
  Linker and Libraries:
    ld='cc'
    ldflags =' -fstack-protector-strong -L/usr/local/lib'
    libpth=/usr/local/lib /usr/lib/x86_64-linux-gnu /usr/lib /lib/x86_64-linux-gnu /lib
    libs=-lpthread -lnsl -lgdbm -ldb -ldl -lm -lcrypt -lutil -lc -lgdbm_compat
    perllibs=-lpthread -lnsl -ldl -lm -lcrypt -lutil -lc
    libc=libc-2.33.so
    so=so
    useshrplib=false
    libperl=libperl.a
    gnulibc_version='2.33'
  Dynamic Linking:
    dlsrc=dl_dlopen.xs
    dlext=so
    d_dlsymun=undef
    ccdlflags='-Wl,-E'
    cccdlflags='-fPIC'
    lddlflags='-shared -O2 -L/usr/local/lib -fstack-protector-strong'


---
@INC for perl 5.37.3:
    lib
    /usr/local/lib/perl5/site_perl/5.37.3/x86_64-linux
    /usr/local/lib/perl5/site_perl/5.37.3
    /usr/local/lib/perl5/5.37.3/x86_64-linux
    /usr/local/lib/perl5/5.37.3

---
Environment for perl 5.37.3:
    HOME=/home/ntyni
    LANG=C
    LANGUAGE (unset)
    LC_CTYPE=fi_FI.UTF-8
    LD_LIBRARY_PATH (unset)
    LOGDIR (unset)
    PATH=/usr/local/bin:/usr/bin:/bin:/usr/games
    PERL_BADLANG (unset)
    SHELL=/bin/zsh
@ijackson
Copy link

ijackson commented Aug 7, 2022

Another way to exercise this behaviour is this

perl 3>/dev/null -MIO::Handle -e 'open X, "<&3" or die $!; $_ = <X>; printf "%s %s\n", X->error(), $!;'

I looked through /dev and on my system this demonstrtes the malfuntion too:

perl -MIO::Handle -e 'open X, "/dev/video0" or die $!; $_ = <X>; printf "%s %s\n", X->error(), $!;'

(IHNI what /dev/video0 is; it came up near the end of my ls and I observed that cat </dev/video0 >/dev/null was able to open the file but not read it.)

@jkeenan
Copy link
Contributor

jkeenan commented Aug 7, 2022

In Debian 1016369, Damyan Ivanov provided this test program (which I've renamed with the two bug report numbers):

$ cat gh-12782-debian-1016369-io-handle.t
use Test::More;

plan skip_all => "IO::Handle not available" unless eval 'use IO::Handle; 1';

open(X, "<", ".") or die $!;
ok(!defined(<X>), 'failed reading from ".", as expected');
ok(X->error,      'X->error is TRUE after reading from "."');
close X;

SKIP: {
    skip "/dev/full not available", 3 unless -w '/dev/full';

    open X, '>', '/dev/full' or die $!;
    ok((print X "1"), "successful write to /dev/full");
    ok(!X->flush,     "flush after writing to /dev/full fails");
    ok(X->error,      "X->error is TRUE after flushing /dev/full");
    close X;
}

done_testing;

The program fails, but in different ways, on both Linux and FreeBSD.

Linux (Ubuntu 20.04 LTS)

$ prove -v gh-12782-debian-1016369-io-handle.t
gh-12782-debian-1016369-io-handle.t .. 
ok 1 - failed reading from ".", as expected
not ok 2 - X->error is TRUE after reading from "."

#   Failed test 'X->error is TRUE after reading from "."'
#   at gh-12782-debian-1016369-io-handle.t line 7.
ok 3 - successful write to /dev/full
ok 4 - flush after writing to /dev/full fails
ok 5 - X->error is TRUE after flushing /dev/full
1..5
# Looks like you failed 1 test of 5.
Dubious, test returned 1 (wstat 256, 0x100)
Failed 1/5 subtests 

Test Summary Report
-------------------
gh-12782-debian-1016369-io-handle.t (Wstat: 256 (exited 1) Tests: 5 Failed: 1)
  Failed test:  2
  Non-zero exit status: 1
Files=1, Tests=5,  0 wallclock secs ( 0.04 usr  0.02 sys +  0.05 cusr  0.00 csys =  0.11 CPU)
Result: FAIL

FreeBSD (version 12)

$ prove -v gh-12782-debian-1016369-io-handle.t
gh-12782-debian-1016369-io-handle.t .. 
ok 1 - failed reading from ".", as expected
not ok 2 - X->error is TRUE after reading from "."

#   Failed test 'X->error is TRUE after reading from "."'
#   at gh-12782-debian-1016369-io-handle.t line 7.
ok 3 - successful write to /dev/full
ok 4 - flush after writing to /dev/full fails
not ok 5 - X->error is TRUE after flushing /dev/full

#   Failed test 'X->error is TRUE after flushing /dev/full'
#   at gh-12782-debian-1016369-io-handle.t line 16.
1..5
# Looks like you failed 2 tests of 5.
Dubious, test returned 2 (wstat 512, 0x200)
Failed 2/5 subtests 

Test Summary Report
-------------------
gh-12782-debian-1016369-io-handle.t (Wstat: 512 Tests: 5 Failed: 2)
  Failed tests:  2, 5
  Non-zero exit status: 2
Files=1, Tests=5,  0 wallclock secs ( 0.03 usr  0.01 sys +  0.10 cusr  0.00 csys =  0.14 CPU)
Result: FAIL

So our definition of the problem, as well as our solution and the tests we use to verify that solution, will have an OS-specific component.

@jkeenan
Copy link
Contributor

jkeenan commented Aug 8, 2022

The program fails, but in different ways, on both Linux and FreeBSD.

Linux (Ubuntu 20.04 LTS)

And, FWIW, it fails differently with different versions of Perl. On perl-5.32 and earlier, on Linux I get test failures in tests 2 and 5 just as I do on FreeBSD. On perl-5.34 I only get a failure in test 5. Bisecting points to the following commit as causing the improvement.

commit 89341f87f9fc65c4d7133e497bb04586e86b8052
Author: Tony Cook <tony@develop-help.com>
Date:   Tue May 12 10:29:17 2020 +1000

    make $fh->error report errors from both input and output
    
    For character devices and sockets perl uses separate PerlIO objects
    for input and output so they can be buffered separately.
    
    The IO::Handle::error() method only checked the input stream, so
    if a write error occurs error() would still returned false.
    
    Change this so both the input and output streams are checked.
    
    fixes #6799

I don't yet understand exactly how this relates to the OP's problem.

@jkeenan
Copy link
Contributor

jkeenan commented Aug 8, 2022

@tonycoz, @ppisar: You have recently touched ferror in dist/IO/IO.xs. Could you please take a look at this ticket?

@ntyni
Copy link
Contributor Author

ntyni commented Aug 8, 2022

I don't yet understand exactly how this relates to the OP's problem.

The referenced Debian report discusses both a read error issue and a write error issue.

The /dev/full test is for the write error issue. I believe it was #6799 and got fixed in 5.34.

The read error issue is what I filed this bug about.

Hope this clarifies.

@jkeenan jkeenan added dist-IO issues in the dual-life blead-first IO distribution and removed Needs Triage labels Aug 8, 2022
@tonycoz
Copy link
Contributor

tonycoz commented Aug 9, 2022

I see #12782 is very similar but hasn't really received much attention. Is this something that should at least be documented as a caveat if it's not fixable?

I expect it's fixable, but might cause other changes in behaviour, I tried:

diff --git a/pp_hot.c b/pp_hot.c
index 08ddf9d7f6..31e008fb5a 100644
--- a/pp_hot.c
+++ b/pp_hot.c
@@ -3292,11 +3292,12 @@ Perl_do_readline(pTHX)
                 || SNARF_EOF(gimme, PL_rs, io, sv)
                 || PerlIO_error(fp)))
         {
-            PerlIO_clearerr(fp);
             if (IoFLAGS(io) & IOf_ARGV) {
                 fp = nextargv(PL_last_in_gv, PL_op->op_flags & OPf_SPECIAL);
-                if (fp)
+                if (fp) {
+                    PerlIO_clearerr(fp);
                     continue;
+                }
                 (void)do_close(PL_last_in_gv, FALSE);
             }
             else if (type == OP_GLOB) {

which allowed one of your test cases to pass (I didn't try the others):

$ ./perl -Ilib -MIO::Handle -e 'open X, "<", "." or die $!; $_ = <X>; printf "%s %s\n", X->error(), $!;'
1 Is a directory

but caused a test failure I don't have time to track down right now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dist-IO issues in the dual-life blead-first IO distribution
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants