Perl / perl5

🐪 The Perl programming language
https://dev.perl.org/perl5/
Other
1.94k stars 554 forks source link

perlio handling of dup when some STD streams are closed is messed up #7105

Open p5pRT opened 20 years ago

p5pRT commented 20 years ago

Migrated from rt.perl.org#26670 (status was 'open')

Searchable as RT26670$

p5pRT commented 20 years ago

From stas@stason.org

Created by stas@rabbit.stason.org

perlio handling of dup when some STD streams are closed is messed up. Consider the following examples​:

% perl -wle 'close STDOUT; open my $fh\, "\<&STDIN" or die;' Filehandle STDOUT reopened as $fh only for input at -e line 1.

% perl -wle 'close STDIN; open my $fh\, ">&STDOUT" or die;' Filehandle STDIN reopened as $fh only for output at -e line 1.

% perl -wle 'close STDIN; open my $fh\, ">&STDERR" or die;' Filehandle STDIN reopened as $fh only for output at -e line 1.

% perl -wle 'close STDIN; open my $fh\, ">/tmp/foo" or die "$!"; \   open my $fh2\, ">&"\, $fh or die "$!";' Filehandle STDIN reopened as $fh only for output at -e line 1.

All these bogus warnings are coming from doio.c​:539

  538 if (ckWARN(WARN_IO)) {   539 if ((IoTYPE(io) == IoTYPE_RDONLY) &&   540 (fp == PerlIO_stdout() || fp == PerlIO_stderr())) {   541 Perl_warner(aTHX_ packWARN(WARN_IO)\,   542 "Filehandle STD%s reopened as %s only for input"\,   543 ((fp == PerlIO_stdout()) ? "OUT" : "ERR")\,   544 GvENAME(gv));   545 }   546 else if ((IoTYPE(io) == IoTYPE_WRONLY) && fp == PerlIO_stdin()) {   547 Perl_warner(aTHX_ packWARN(WARN_IO)\,   548 "Filehandle STDIN reopened as %s only for output"\,   549 GvENAME(gv));   550 }   551 }

all these cases suffer from the same problem. A dup was taking over one of the STD streams which wasn't used\, as it was closed just before the dup.

A possible solution is try and avoid taking over STD slots by dupping at most 3 times\, and then closing the dupped unused filehandles.

Perl Info ``` Flags: category=core severity=medium Site configuration information for perl v5.8.3: Configured by stas at Wed Jan 14 18:18:11 PST 2004. Summary of my perl5 (revision 5.0 version 8 subversion 3) configuration: Platform: osname=linux, osvers=2.4.22-10mdk, archname=i686-linux-thread-multi uname='linux rabbit.stason.org 2.4.22-10mdk #1 thu sep 18 12:30:58 cest 2003 i686 unknown unknown gnulinux ' config_args='-des -Dprefix=/home/stas/perl/5.8.3-ithread -Dusethreads -Doptimize=-g -Duseshrplib -Dusedevel' hint=recommended, useposix=true, d_sigaction=define usethreads=define use5005threads=undef useithreads=define usemultiplicity=define useperlio=define d_sfio=undef uselargefiles=define usesocks=undef use64bitint=undef use64bitall=undef uselongdouble=undef usemymalloc=n, bincompat5005=undef Compiler: cc='cc', ccflags ='-D_REENTRANT -D_GNU_SOURCE -DTHREADS_HAVE_PIDS -DDEBUGGING -fno-strict-aliasing -I/usr/local/include -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64 -I/usr/include/gdbm', optimize='-g', cppflags='-D_REENTRANT -D_GNU_SOURCE -DTHREADS_HAVE_PIDS -DDEBUGGING -fno-strict-aliasing -I/usr/local/include -I/usr/include/gdbm' ccversion='', gccversion='3.3.2 (Mandrake Linux 10.0 3.3.2-4mdk)', gccosandvers='' intsize=4, longsize=4, ptrsize=4, doublesize=8, byteorder=1234 d_longlong=define, longlongsize=8, d_longdbl=define, longdblsize=12 ivtype='long', ivsize=4, nvtype='double', nvsize=8, Off_t='off_t', lseeksize=8 alignbytes=4, prototype=define Linker and Libraries: ld='cc', ldflags =' -L/usr/local/lib' libpth=/usr/local/lib /lib /usr/lib libs=-lnsl -lgdbm -ldb -ldl -lm -lcrypt -lutil -lpthread -lc perllibs=-lnsl -ldl -lm -lcrypt -lutil -lpthread -lc libc=/lib/libc-2.3.2.so, so=so, useshrplib=true, libperl=libperl.so gnulibc_version='2.3.2' Dynamic Linking: dlsrc=dl_dlopen.xs, dlext=so, d_dlsymun=undef, ccdlflags='-rdynamic -Wl,-rpath,/home/stas/perl/5.8.3-ithread/lib/5.8.3/i686-linux-thread-multi/CORE' cccdlflags='-fpic', lddlflags='-shared -L/usr/local/lib' Locally applied patches: @INC for perl v5.8.3: /home/stas/perl/5.8.3-ithread/lib/5.8.3/i686-linux-thread-multi /home/stas/perl/5.8.3-ithread/lib/5.8.3 /home/stas/perl/5.8.3-ithread/lib/site_perl/5.8.3/i686-linux-thread-multi /home/stas/perl/5.8.3-ithread/lib/site_perl/5.8.3 /home/stas/perl/5.8.3-ithread/lib/site_perl . Environment for perl v5.8.3: HOME=/home/stas LANG=en_GB LANGUAGE=en_GB:en LC_ADDRESS=en_CA LC_COLLATE=en_GB LC_CTYPE=en_GB LC_IDENTIFICATION=en_CA LC_MEASUREMENT=en_CA LC_MESSAGES=en_GB LC_MONETARY=en_CA LC_NAME=en_CA LC_NUMERIC=en_CA LC_PAPER=en_CA LC_TELEPHONE=en_CA LC_TIME=en_GB LD_LIBRARY_PATH (unset) LOGDIR (unset) PATH=/usr//bin:/bin:/usr/bin:.:/usr/local/bin:/usr/X11R6/bin:/usr/games:/home/stas/bin:/home/stas/bin:/usr/local/bin:/usr/X11R6/bin:/usr/java/j2re1.4.0/bin/ PERLDOC_PAGER=less -R PERL_BADLANG (unset) SHELL=/bin/tcsh __________________________________________________________________ Stas Bekman JAm_pH ------> Just Another mod_perl Hacker http://stason.org/ mod_perl Guide ---> http://perl.apache.org mailto:stas@stason.org http://use.perl.org http://apacheweek.com http://modperlbook.org http://apache.org http://ticketmaster.com ```
p5pRT commented 20 years ago

From nick.ing-simmons@elixent.com

Stas Bekman \perl5\-porters@&#8203;perl\.org writes​:

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

This is a bug report for perl from stas@​rabbit.stason.org\, generated with the help of perlbug 1.34 running under perl v5.8.3.

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

perlio handling of dup when some STD streams are closed is messed up. Consider the following examples​:

% perl -wle 'close STDOUT; open my $fh\, "\<&STDIN" or die;' Filehandle STDOUT reopened as $fh only for input at -e line 1.

$fh now has fd=1 and _is_ stdout and hence STDOUT.

% perl -wle 'close STDIN; open my $fh\, ">&STDOUT" or die;' Filehandle STDIN reopened as $fh only for output at -e line 1.

% perl -wle 'close STDIN; open my $fh\, ">&STDERR" or die;' Filehandle STDIN reopened as $fh only for output at -e line 1.

% perl -wle 'close STDIN; open my $fh\, ">/tmp/foo" or die "$!"; \ open my $fh2\, ">&"\, $fh or die "$!";' Filehandle STDIN reopened as $fh only for output at -e line 1.

All these bogus warnings are coming from doio.c​:539

They are NOT bogus. We have reworded the warning a few times to try and make it say what the problem is.

538 if (ckWARN(WARN_IO)) { 539 if ((IoTYPE(io) == IoTYPE_RDONLY) && 540 (fp == PerlIO_stdout() || fp == PerlIO_stderr())) { 541 Perl_warner(aTHX_ packWARN(WARN_IO)\, 542 "Filehandle STD%s reopened as %s only for input"\, 543 ((fp == PerlIO_stdout()) ? "OUT" : "ERR")\, 544 GvENAME(gv)); 545 } 546 else if ((IoTYPE(io) == IoTYPE_WRONLY) && fp == PerlIO_stdin()) { 547 Perl_warner(aTHX_ packWARN(WARN_IO)\, 548 "Filehandle STDIN reopened as %s only for output"\, 549 GvENAME(gv)); 550 } 551 }

all these cases suffer from the same problem. A dup was taking over one of the STD streams which wasn't used\, as it was closed just before the dup.

Yes. But it isn't just dup but other open()s as well.

A possible solution is try and avoid taking over STD slots

There is a lot of legacy code that relies on taking over STD slots. But in general that code makes sure that fd==0 is readable and fd==1 and fd==2 are writable. So don't solicit the warning.

by dupping at most 3 times\, and then closing the dupped unused filehandles.

Patches that don't break 'make test' on any of BSD-ish/SVR4-ish/Win32 welcome. In particular IPC​::Open3 must continue to work.

p5pRT commented 20 years ago

The RT System itself - Status changed from 'new' to 'open'

p5pRT commented 20 years ago

From perl5-porters@ton.iguana.be

For what it's worth\, I've ran into this warning several times and it was bogus every time. Either I WANTED to dup to fd 1 with a reader (often makes sense with the qmail components)\, or I didn't CARE (e.g. I already had STDOUT closed for a later dup\, but first I needed to read something from somewhere). (it even dumped core in some old perl around 5.7.0 where it wanted to always send this message to stderr which was also closed).

But skipping the STD handles could be even worse. In e.g. the qmail case I *must* dup a reader to filehandle 1.

p5pRT commented 20 years ago

From stas@stason.org

Nick Ing-Simmons via RT wrote​: [...]

A possible solution is try and avoid taking over STD slots

There is a lot of legacy code that relies on taking over STD slots. But in general that code makes sure that fd==0 is readable and fd==1 and fd==2 are writable. So don't solicit the warning.

I understand\, Nick

But take the last test case​:

% perl -wle 'close STDIN; open my $fh\, ">/tmp/foo" or die "$!"; \   open my $fh2\, ">&"\, $fh or die "$!";'   Filehandle STDIN reopened as $fh only for output at -e line 1.

and move that 'close STDIN' out. Which leaves us with a perfectly valid code\, right?

  open my $fh\, ">/tmp/foo" or die "$!";   open my $fh2\, ">&"\, $fh or die "$!";'

Now if some other code (which I have no control of) closes STDIN\, my perfectly valid code suddenly doesn't work. In that example I don't care about any of the STD streams\, all I want is to open a filehandle and then dup it. What am I supposed to do here?

So may be the warnings are not bogus\, but the dup function is.

__________________________________________________________________ Stas Bekman JAm_pH ------> Just Another mod_perl Hacker http​://stason.org/ mod_perl Guide ---> http​://perl.apache.org mailto​:stas@​stason.org http​://use.perl.org http​://apacheweek.com http​://modperlbook.org http​://apache.org http​://ticketmaster.com

p5pRT commented 20 years ago

From nick.ing-simmons@elixent.com

Stas Bekman \stas@&#8203;stason\.org writes​:

Nick Ing-Simmons via RT wrote​: [...]

A possible solution is try and avoid taking over STD slots

There is a lot of legacy code that relies on taking over STD slots. But in general that code makes sure that fd==0 is readable and fd==1 and fd==2 are writable. So don't solicit the warning.

I understand\, Nick

But take the last test case​:

% perl -wle 'close STDIN; open my $fh\, ">/tmp/foo" or die "$!"; \ open my $fh2\, ">&"\, $fh or die "$!";' Filehandle STDIN reopened as $fh only for output at -e line 1.

and move that 'close STDIN' out. Which leaves us with a perfectly valid code\, right?

open my $fh\, ">/tmp/foo" or die "$!"; open my $fh2\, ">&"\, $fh or die "$!";'

Now if some other code (which I have no control of) closes STDIN\, my perfectly valid code suddenly doesn't work.

There are lots of other things code you have no control over can do to stop your code working.

In that example I don't care about any of the STD streams\, all I want is to open a filehandle and then dup it. What am I supposed to do here?

Find the person that wrote the code that closed STDIN without reopening to /dev/null and give 'em hell ;-)

Perhaps run

foreach my $fh ((\*STDIN\,\*STDOUT\,\*STDERR)) {   open($fh\,">+/dev/null") unless defined fileno($fh); }

After dubious code is allowed to run?

The snag is we can't warn on close(STD*) as that is normal. So we warn when the re-open looks wrong.

Perhaps we could warn on leaving scope with STD* closed? So that   {   ...   close(STD)   ...   open(STD\,...)   } # ok

  {   ...   close(STD)   } # warn

So may be the warnings are not bogus\, but the dup function is.

In your example it is the open() not the dup that is warning.

perl -wle 'close STDIN; open my $fh\, ">/tmp/foo" or die "$!";'

p5pRT commented 20 years ago

From @rgs

Nick Ing-Simmons wrote​:

Find the person that wrote the code that closed STDIN without reopening to /dev/null and give 'em hell ;-)

I wholehearledly agree (from personal experience as a C coder...)

p5pRT commented 20 years ago

From stas@stason.org

Nick Ing-Simmons via RT wrote​:

In that example I don't care about any of the STD streams\, all I want is to open a filehandle and then dup it. What am I supposed to do here?

Find the person that wrote the code that closed STDIN without reopening to /dev/null and give 'em hell ;-)

That person could be me ;) I remember reading a lot about always closing STD* streams immediatelly after fork\, but I don't remember someone mentioning that they need to be re-opened to /dev/null.

And it must be File​::Spec->devnull() ;)

So should I fix the fork examples to reopen the streams to /dev/null? http​://perl.apache.org/docs/1.0/guide/performance.html#Freeing_the_Parent_Process

I've grepped the perl pod dir and haven't found any problematic cases. The only place it suggests to 'close STDOUT' is with inmemory fhs and I've tested that this works w/o a warning as they are both re-opened.

use warnings; my $var = "foo"; close STDIN; open STDIN\, "\<"\, \$var or die "Can't open STDIN​: $!"; close STDOUT; open STDOUT\, ">"\, \$var or die "Can't open STDOUT​: $!"; open my $fh\, ">/tmp/foo" or die "$!";

But consider this​:

use warnings; local $\ = "\n"; close STDOUT; open STDOUT\, ">"\, \my $var or die "Can't open STDOUT​: $!"; qx[echo -n foo > /tmp/foo]; open my $fh\, "\</tmp/foo" or die "$!"; print STDERR fileno($fh); print STDERR \<$fh>;

prints​:

1 foo

As you can see $fh got fd 1\, which is STDOUT. Though $fh is open for RDONLY. Why there is no warning?

The following is unclear​:

use warnings; local $\ = "\n"; my $var = "foo"; close STDIN; open STDIN\, "\<"\, \$var or die "Can't open STDIN​: $!"; print \; open my $fh\, ">/tmp/foo" or die "$!"; print STDERR fileno($fh);

it prints​:

foo 4

why 4 and not 0? after all I have closed STDIN and then re-opened it using an in-memory fh. It did work\, as it printed 'foo'\, but a new open got fd 4.

Perhaps run

foreach my $fh ((\*STDIN\,\*STDOUT\,\*STDERR)) { open($fh\,">+/dev/null") unless defined fileno($fh); }

After dubious code is allowed to run?

I guess that's doable if you are unfortunate to find yourself in this kind of situation.

The snag is we can't warn on close(STD*) as that is normal. So we warn when the re-open looks wrong.

Could you look at GP and if its name is not STD\, do another open and then close\, to make sure you get a fd above 2? Earlier you said that some some people use that as a feature. Do they always use STD(IN|OUT) fh or it can be anything?

Perhaps we could warn on leaving scope with STD* closed? So that { ... close(STD) ... open(STD\,...) } # ok

{ ... close(STD) } # warn

That sounds like a good idea to me. What about entering a new scope with STDs closed? But both are probably are too much of an overhead for the normal case.

So may be the warnings are not bogus\, but the dup function is.

In your example it is the open() not the dup that is warning.

perl -wle 'close STDIN; open my $fh\, ">/tmp/foo" or die "$!";'

Yes\, yes\, you are right.

As usual\, Thanks a lot\, Nick!

__________________________________________________________________ Stas Bekman JAm_pH ------> Just Another mod_perl Hacker http​://stason.org/ mod_perl Guide ---> http​://perl.apache.org mailto​:stas@​stason.org http​://use.perl.org http​://apacheweek.com http​://modperlbook.org http​://apache.org http​://ticketmaster.com