[perl #57512] Implicit close()s are silently unchecked for error

# New Ticket Created by  tchrist1 
# Please include the string:  [perl #57512]
# in the subject line of all future correspondence about this issue. 
# <URL: http://rt.perl.org/rt3/Ticket/Display.html?id=57512 >



This is a bug report for perl from tchrist@chthon.perl.com,
generated with the help of perlbug 1.36 running under perl 5.10.0.


-----------------------------------------------------------------
[Please enter your report here]

  ( Perhaps the severity of this bug should be "high",
    not "medium"?  Feel free to upgrade the status if you
    find yourself agreeing with me. )

When Perl implicitly closes a filehandle, it neglects to detect and then
duly notify the user of having failed to do so correctly.  This leads to
potentially severe errors going undetected.  It needs to be fixed.

Perl implicitly closes a filehandle under the following circumstances:

    (1) When you reopen a handle that's already open. The return
        value reflects only the success of the open, not that of a
        failed close.  This may include the ARGVOUT (perl -I, $^I)
        situation, which also goes astonishingly unchecked for error,
        surely a mistake.

    (2) When an implicitly allocated but still open filehandle 
        gets GC'd.  This includes both

	    open(my $fh, "> /tmp/foo.$$");  # please use File::Temp

	style filehandles as well as those in the localized 
	typeglob, such as:

	    local *FH;

    (3) Prior to certain critical operations, such as fork(), flock(),
        and perhaps seek()?, where you don't want a dirty or duplicate
        I/O buffer, an implicit fflush() executes.  The close-on-exec
        flag will close them at exec(), but they needed to be already
        flushed at fork() time.

    (4) During global destruction, Perl chases down all the open handles
        and fflush()es and fclose()s them.  Cf exit() vs _exit() in
        libc.  This failure to report errors is most egregious on
        STDOUT, as every program that needs to operate correctly (and
        which ones don't?) needs must install

	    END { close(STDOUT) || die "can't close STDOUT: $!" }

        Since this is all-but-compulsary on any program you want to
        behave correctly, it needs to be in all programs; hence, it
        needs to be in the run-time.

It is not obvious the nature of the warning or error to report. Some
situations may call for some level, others for others. It is possible
that all but the final case should be a warning only.  I tend to think
this should be of class (S io), that is,  a severe, on-by-default
warning of class io.

However, the global destruction errors, *particularly* of STDOUT, are
something else.  I believe that a failure to close STDOUT at that point
(maybe at 2, too?) should and must cause the program's exit status to
be non-zero, for it has failed just as much as a broken-piped program
has failed, and *those* program die of SIGPIPE, exiting thus 141, not 0.

Below is the relevant portion of the originating discussion of this that
I sent to p5p.  In it, you see how when the cat program fails in this
way, it indeed causes the very program to fail.  I feel perl should do
at least as much.

    Subject: Re: Creative and *routine* use of so-called "magic" ARGV (was [perl #2783] Security of ARGV using 2-argument open) 
    In-Reply-To: Message from Abigail <abigail@abigail.be> 
       of "Tue, 29 Jul 2008 09:29:13 +0200." <20080729072913.GM30221@almanda> 
    Date: Tue, 29 Jul 2008 15:19:34 -0600
    Message-ID: <29912.1217366374@chthon>
    From: Tom Christiansen <tchrist@chthon>

Watch:

    % df -h .
    Filesystem     Size    Used   Avail Capacity  Mounted on
    /dev/wd0a      124M    117M   -5.0K   100%    /

    % ls -l it*
    -rw-r--r--  1 tchrist  wheel  0 Jul 29 14:06 it

    % perl -i.orig -pe 'print "this is more stuff\n"' it

    % echo $?
    0

    % ls -l it*
    0 -rw-r--r--  1 tchrist  wheel  0 Jul 29 15:05 it
    0 -rw-r--r--  1 tchrist  wheel  0 Jul 29 14:06 it.orig

To this day, Perl's implicit closing of files doesn't warn you of
errors, let alone exit nonzero.  This makes it do wrong thing and
not even tell you it did them wrong.  This is a *true* problem,
because checking for the success of print() is neither necessary
nor sufficient to detect the success of print().  Yes, you read
that correctly.  It's because of buffering, plus the persistence
of the err flag on the file structure.

I've never convinced anybody this is important.  Since *every*
program should do this for correctness, it has to be in the run-time
system to avoid it ever being forgotten.  Sure, there's stuff like
this you can do:

    END { close(STDOUT) || die "can't close stdout: $!" }

But that sort of thing should happen on all implicitly closed things.  And
it really must.  Even IO::Handle::close doesn't bother.  Perl knows what
handles it's flushing closing during global destruction, at least if you
don't just blindly fflush(0).

Watch again, starting from before the bogus, ill-reported rename attempt:

    % df -h .
    Filesystem     Size    Used   Avail Capacity  Mounted on
    /dev/wd0a      124M    117M   -5.0K   100%    /

    % ls -l it
    -rw-r--r--  1 tchrist  wheel  0 Jul 29 14:06 it

    % perl -e 'print "stuff\n"' >> it; echo $?                      
    0

    % perl -e 'open(my $fh, ">>it") || die "open $!"; print $fh "stuff\n"; print STDOUT "ok now\n"'
    ok now

    % echo $?
    0

    % ls -l it
    -rw-r--r--  1 tchrist  wheel  0 Jul 29 14:06 it

This is all incorrect behavior on Perl's part.  Even cat knows better!

    % echo foo | cat >> it; echo $?
    cat: stdout: No space left on device
    1

+-------------------------------------------------------+
| Notice how even my cat is smarter than your perl!? :( |
+-------------------------------------------------------+

What's that about, eh?

But I've been saying this for years, just like everything else.
Makes no difference.  Depressing, eh?

BTW, this proves my point about checking for print's status
being a waste of time, neither necessary nor sufficient to 
catch failed prints!

    % perl -e 'open(my $fh, ">>it") || die "open $!"; print $fh "stuff\n" or die "print $!"; print STDOUT "ok now\n"'; echo exit status was $?
    ok now
    exit status was 0

And you can't get the danged thing to detect its folly:

    % perl -WE 'open(my $fh, ">>it") || die "open $!"; say $fh "stuff" or die "print $!"; say "ok now"' ; echo exit status was $?
    ok now
    exit status was 0

I firmly believe that this needs to be a part of *EVERY* Perl program,
which consequently means it should *not* be there, but in the core itself:

    END { close(STDOUT) || die "can't close stdout: $!" }

And I believe this *much* more than some here believe <ARGV> needs
to implicitly map {"< $_\0"} @ARGV (since that breaks existing
working code, whereas mine fixes existing broken code).  Furthermore,
I also believe all *IMPLICIT* closes that fail need to generate a
mandatory io warning, but that of STDOUT should be a fatal.  I've
believed all this for a long time; said it often enough.  Anything
else is just plain wrong behavior.  I'm not quite sure whether
ARGVOUT failure should be a warning or a fatal, but it should be
*something* suitably noisy.

[Please do not change anything below this line]
-----------------------------------------------------------------
---
Flags:
    category=core
    severity=medium
---
Site configuration information for perl 5.10.0:

Configured by tchrist at Mon May 19 00:58:33 MDT 2008.

Summary of my perl5 (revision 5 version 10 subversion 0) configuration:
  Platform:
    osname=openbsd, osvers=3.7, archname=OpenBSD.i386-openbsd-thread-multi-64int
    uname='openbsd chthon 3.7 generic#50 i386 '
    config_args='-Dusethreads -Duse64bitint -O -des'
    hint=recommended, useposix=true, d_sigaction=define
    useithreads=define, usemultiplicity=define
    useperlio=define, d_sfio=undef, uselargefiles=define, usesocks=undef
    use64bitint=define, use64bitall=undef, uselongdouble=undef
    usemymalloc=n, bincompat5005=undef
  Compiler:
    cc='cc', ccflags ='-pthread -fno-strict-aliasing -pipe -I/usr/local/include',
    optimize='-O2',
    cppflags='-pthread -fno-strict-aliasing -pipe -I/usr/local/include'
    ccversion='', gccversion='3.3.5 (propolice)', gccosandvers='openbsd3.7'
    intsize=4, longsize=4, ptrsize=4, doublesize=8, byteorder=12345678
    d_longlong=define, longlongsize=8, d_longdbl=define, longdblsize=12
    ivtype='long long', ivsize=8, nvtype='double', nvsize=8, Off_t='off_t', lseeksize=8
    alignbytes=4, prototype=define
  Linker and Libraries:
    ld='cc', ldflags ='-pthread -Wl,-E  -L/usr/local/lib'
    libpth=/usr/local/lib /usr/lib
    libs=-lm -lutil -lc
    perllibs=-lm -lutil -lc
    libc=/usr/lib/libc.so.34.2, so=so, useshrplib=false, libperl=libperl.a
    gnulibc_version=''
  Dynamic Linking:
    dlsrc=dl_dlopen.xs, dlext=so, d_dlsymun=undef, ccdlflags=' '
    cccdlflags='-DPIC -fPIC ', lddlflags='-shared -fPIC  -L/usr/local/lib'

Locally applied patches:
    

---
@INC for perl 5.10.0:
    /usr/local/lib/perl5/5.10.0/OpenBSD.i386-openbsd-thread-multi-64int
    /usr/local/lib/perl5/5.10.0
    /usr/local/lib/perl5/site_perl/5.10.0/OpenBSD.i386-openbsd-thread-multi-64int
    /usr/local/lib/perl5/site_perl/5.10.0
    /usr/local/lib/perl5/site_perl/5.8.7
    /usr/local/lib/perl5/site_perl/5.8.0
    /usr/local/lib/perl5/site_perl/5.6.0
    /usr/local/lib/perl5/site_perl/5.005
    /usr/local/lib/perl5/site_perl
    .

---
Environment for perl 5.10.0:
    HOME=/home/tchrist
    LANG (unset)
    LANGUAGE (unset)
    LD_LIBRARY_PATH=/usr/local/lib:/usr/lib:/usr/lib/X11:/usr/openwin/lib:/mnt/usr/local/lib:/mnt/usr/lib:/mnt/usr/lib/X11:/mnt/usr/openwin/lib
    LOGDIR (unset)
    PATH=/home/tchrist/scripts:/usr/local/etc:/usr/local/bin:/usr/local/sbin:/etc:/bin:/sbin:/usr/bin:/usr/sbin:/usr/local/games:/usr/local/apache/bin:/usr/games:/usr/X11R6/bin:.
    PERL_BADLANG (unset)
    SHELL=/bin/tcsh

• [perl #57512] Implicit close()s are silently unchecked for error by tchrist1