Front page | perl.perl5.porters |
Postings from August 2008
[perl #57512] Implicit close()s are silently unchecked for error
From:
tchrist1
Date:
August 2, 2008 04:32
Subject:
[perl #57512] Implicit close()s are silently unchecked for error
Message ID:
rt-3.6.HEAD-29759-1217613762-665.57512-75-0@perl.org
# 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