open2 oddity with eval "..." and STDIN closed

p5pRT commented 19 years ago

Migrated from rt.perl.org#37033 (status was 'resolved')

Searchable as RT37033$

p5pRT commented 19 years ago

From pajas@matfyz.cz

This is a bug report for perl from pajas@matfyz.cz\, generated with the help of perlbug 1.35 running under perl v5.8.6.

Hi!

A virtual beer to anyone who sheds some light on this:-)

I'm puzzled by a weird behavior of output redirection in a larger application of mine. Finally\, I was able to trim the code down to the few lines below.

__CODE_STARTS_HERE__ #!/usr/bin/env perl close STDIN; eval \<\<'EVAL'; # \<- remove or replace with eval {} # to see the correct output use Encode::Byte; # \<- instead of POD of this use IPC::Open2; open2('>&STDOUT'\,$P\,'cat') || die "cannot open pipe: $!\n"; print $P ("HALLO\n"); EVAL __END__

It is now only a way round to pass the string "HALLO\n" to 'cat' program. However\, under the specific scenario\, the content that follows __END__ in the module Encode/Byte.pm is unexpectedly passed to 'cat' instead.

HELLO is printed if either of the following conditions is satisfied:

* eval "" is removed * eval "" is replaced with eval { } * the initial close STDIN is removed * close STDIN is moved into the eval

If 'use Encode::Byte' is removed\, the program outputs nothing.

I'm assuming this is a bug because I don't see a reason why changing eval"" to eval{} should make any difference here and also why closing STDIN should be relevant.

I'm marking the bug as medium severe because of the fact that 'use Module' exposes an open filehandle (fd0) with an unexpected content and open2 passes it to a subprocess where it shouldn't.

Beside the reported versions\, I was able to reproduce the problem with a 5.9.3 build on Fedora Core 3.

Thanks\,

-- Petr

Flags: category=core severity=medium

This perlbug was built using Perl v5.8.6 - Sat Mar 19 17:33:54 UTC 2005 It is being executed now by Perl v5.8.6 - Sat Mar 19 17:29:44 UTC 2005.

Site configuration information for perl v5.8.6:

Configured by abuild at Sat Mar 19 17:29:44 UTC 2005.

Summary of my perl5 (revision 5 version 8 subversion 6) configuration: Platform: osname=linux\, osvers=2.6.9\, archname=i586-linux-thread-multi uname='linux salieri 2.6.9 #1 smp fri jan 14 15:41:33 utc 2005 i686 athlon i386 gnulinux ' config_args='-ds -e -Dprefix=/usr -Dvendorprefix=/usr -Dinstallusrbinperl -Dusethreads -Di_db -Di_dbm -Di_ndbm -Di_gdbm -Duseshrplib=true -Doptimize=-O2 -march=i586 -mcpu=i686 -fmessage-length=0 -Wall -g -Wall -pipe' 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 -pipe -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64'\, optimize='-O2 -march=i586 -mcpu=i686 -fmessage-length=0 -Wall -g -Wall -pipe'\, cppflags='-D_REENTRANT -D_GNU_SOURCE -DTHREADS_HAVE_PIDS -DDEBUGGING -fno-strict-aliasing -pipe' ccversion=''\, gccversion='3.3.5 20050117 (prerelease) (SUSE Linux)'\, 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 ='' libpth=/lib /usr/lib /usr/local/lib libs=-lnsl -ldl -lm -lcrypt -lutil -lpthread -lc perllibs=-lnsl -ldl -lm -lcrypt -lutil -lpthread -lc libc=\, so=so\, useshrplib=true\, libperl=libperl.so gnulibc_version='2.3.4' Dynamic Linking: dlsrc=dl_dlopen.xs\, dlext=so\, d_dlsymun=undef\, ccdlflags='-Wl\,-E -Wl\,-rpath\,/usr/lib/perl5/5.8.6/i586-linux-thread-multi/CORE' cccdlflags='-fPIC'\, lddlflags='-shared'

Locally applied patches:

@INC for perl v5.8.6: /usr/lib/perl5/5.8.6/i586-linux-thread-multi /usr/lib/perl5/5.8.6 /usr/lib/perl5/site_perl/5.8.6/i586-linux-thread-multi /usr/lib/perl5/site_perl/5.8.6 /usr/lib/perl5/site_perl /usr/lib/perl5/vendor_perl/5.8.6/i586-linux-thread-multi /usr/lib/perl5/vendor_perl/5.8.6 /usr/lib/perl5/vendor_perl .

Environment for perl v5.8.6: HOME=/home/pajas LANG=cs_CZ.UTF-8 LANGUAGE (unset) LD_LIBRARY_PATH (unset) LOGDIR (unset)
PATH=/home/pajas/bin:/usr/local/bin:/usr/bin:/usr/X11R6/bin:/bin:/usr/games:/opt/gnome/bin:/opt/kde3/bin PERL_BADLANG (unset) SHELL=/bin/bash

p5pRT commented 12 years ago

From @jkeenan

On Tue Aug 30 12:04:45 2005\, pajas wrote:

This is a bug report for perl from pajas@matfyz.cz\, generated with the help of perlbug 1.35 running under perl v5.8.6.

A virtual beer to anyone who sheds some light on this:-)

I'm puzzled by a weird behavior of output redirection in a larger application of mine. Finally\, I was able to trim the code down to the few lines below.

__CODE_STARTS_HERE__ #!/usr/bin/env perl close STDIN; eval \<\<'EVAL'; # \<- remove or replace with eval {} # to see the correct output use Encode::Byte; # \<- instead of POD of this use IPC::Open2; open2('>&STDOUT'\,$P\,'cat') || die "cannot open pipe: $!\n"; print $P ("HALLO\n"); EVAL __END__

It is now only a way round to pass the string "HALLO\n" to 'cat' program. However\, under the specific scenario\, the content that follows __END__ in the module Encode/Byte.pm is unexpectedly passed to 'cat' instead.

HELLO is printed if either of the following conditions is satisfied:

* eval "" is removed * eval "" is replaced with eval { } * the initial close STDIN is removed * close STDIN is moved into the eval

If 'use Encode::Byte' is removed\, the program outputs nothing.

I'm assuming this is a bug because I don't see a reason why changing eval"" to eval{} should make any difference here and also why closing STDIN should be relevant.

I'm marking the bug as medium severe because of the fact that 'use Module' exposes an open filehandle (fd0) with an unexpected content and open2 passes it to a subprocess where it shouldn't.

Beside the reported versions\, I was able to reproduce the problem with a 5.9.3 build on Fedora Core 3.

And I can reproduce the different variants with Perl 5.14.2 on Darwin.

jimk

p5pRT commented 12 years ago

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

p5pRT commented 12 years ago

From @iabyn

On Tue\, Jan 03\, 2012 at 05:58:33PM -0800\, James E Keenan via RT wrote:

On Tue Aug 30 12:04:45 2005\, pajas wrote:

This is a bug report for perl from pajas@matfyz.cz\, generated with the help of perlbug 1.35 running under perl v5.8.6.

A virtual beer to anyone who sheds some light on this:-)

I'm puzzled by a weird behavior of output redirection in a larger application of mine. Finally\, I was able to trim the code down to the few lines below.

__CODE_STARTS_HERE__ #!/usr/bin/env perl close STDIN; eval \<\<'EVAL'; # \<- remove or replace with eval {} # to see the correct output use Encode::Byte; # \<- instead of POD of this use IPC::Open2; open2('>&STDOUT'\,$P\,'cat') || die "cannot open pipe: $!\n"; print $P ("HALLO\n"); EVAL __END__

It is now only a way round to pass the string "HALLO\n" to 'cat' program. However\, under the specific scenario\, the content that follows __END__ in the module Encode/Byte.pm is unexpectedly passed to 'cat' instead.

HELLO is printed if either of the following conditions is satisfied:

* eval "" is removed * eval "" is replaced with eval { } * the initial close STDIN is removed * close STDIN is moved into the eval

If 'use Encode::Byte' is removed\, the program outputs nothing.

I'm assuming this is a bug because I don't see a reason why changing eval"" to eval{} should make any difference here and also why closing STDIN should be relevant.

I'm marking the bug as medium severe because of the fact that 'use Module' exposes an open filehandle (fd0) with an unexpected content and open2 passes it to a subprocess where it shouldn't.

Beside the reported versions\, I was able to reproduce the problem with a 5.9.3 build on Fedora Core 3.

And I can reproduce the different variants with Perl 5.14.2 on Darwin.

It's not a bug. The eval is only significant in that it allows a 'use' to happen *after* STDIN has been closed.

A similar effect can be shown here:

BEGIN { close STDIN; } use strict; while (\<>) { chomp; print "[$_]\n"; }

which prints out all the lines in lib/strict.pm following the __END__.

When perl opens a source file\, it uses the next available file descriptor -- which happens to be 0 in this case\, since STDIN was closed. Since the source file contains an __END__\, perl keeps the source file open\, in case of a later \<strict::DATA>. So reading from STDIN (aka fd 0) reads from the text following __END__.

-- Diplomacy is telling someone to go to hell in such a way that they'll look forward to the trip

p5pRT commented 12 years ago

From @jkeenan

On 1/8/12 9:56 AM\, Dave Mitchell wrote:

It's not a bug. The eval is only significant in that it allows a 'use' to happen *after* STDIN has been closed.

A similar effect can be shown here:
 BEGIN \{ close STDIN; \}
 use strict;
 while $\<>$ \{
chomp;
print "\[$\_\]\\n";
 \}
which prints out all the lines in lib/strict.pm following the __END__.

When perl opens a source file\, it uses the next available file descriptor -- which happens to be 0 in this case\, since STDIN was closed. Since the source file contains an __END__\, perl keeps the source file open\, in case of a later\<strict::DATA>. So reading from STDIN (aka fd 0) reads from the text following __END__.

Is there anything which should be added to the Perl documentation with respect to this?

Otherwise\, we should close the ticket.

Thank you very much. Jim Keenan

p5pRT commented 12 years ago

From @rjbs

I think this falls under the category of "not to be documented." It's standard fd behavior\, and while it can be surprising\, we can either document it in one place that won't be helpful or we can
put documentation all over the place that will just confuse readers.

I think this is a pretty rare scenario that is actually going to be best helped by the user asking what's up.

tl;dr\, some edges are best left explained anew each time

p5pRT commented 12 years ago

From [Unknown Contact. See original ticket]

I think this falls under the category of "not to be documented." It's standard fd behavior\, and while it can be surprising\, we can either document it in one place that won't be helpful or we can
put documentation all over the place that will just confuse readers.

I think this is a pretty rare scenario that is actually going to be best helped by the user asking what's up.

tl;dr\, some edges are best left explained anew each time

p5pRT commented 12 years ago

@rjbs - Status changed from 'open' to 'rejected'

p5pRT commented 12 years ago

From @nwc10

On Sun\, Jan 08\, 2012 at 02:56:12PM +0000\, Dave Mitchell wrote:

It's not a bug.

I think it *is* a bug :-)

The eval is only significant in that it allows a 'use' to happen *after* STDIN has been closed.

A similar effect can be shown here:
BEGIN \{ close STDIN; \}
use strict;
while $\<>$ \{
chomp;
print "\[$\_\]\\n";
\}
which prints out all the lines in lib/strict.pm following the __END__.

When perl opens a source file\, it uses the next available file descriptor -- which happens to be 0 in this case\, since STDIN was closed. Since the source file contains an __END__\, perl keeps the source file open\, in case of a later \<strict::DATA>. So reading from STDIN (aka fd 0) reads from the text following __END__.

perldata says:

Text after __DATA__ may be read via the filehandle C\<PACKNAME::DATA>\, where C\ is the package that was current when the __DATA__ token was encountered. The filehandle is left open pointing to the line after __DATA__. It is the program's responsibility to C\ when it is done reading from it. For compatibility with older scripts written before __DATA__ was introduced\, __END__ behaves like __DATA__ in the top level script (but not in files loaded with C\ or C\) and leaves the remaining contents of the file accessible via C\<main::DATA>.

ie the behavour of leaving the file handle open would be correct for __DATA__ But this is __END__\, and it's not the top level script\, so it should be closed.

Contrast what happens when one causes file descriptor 1 to be the one used to read lib/strict.pm:

$ strace -eopen\,close ./perl -Ilib -e 'close STDOUT; eval "use strict"; ' open("/etc/ld.so.cache"\, O_RDONLY) = 3 close(3) = 0 open("/lib/i686/cmov/libnsl.so.1"\, O_RDONLY) = 3 close(3) = 0 open("/lib/i686/cmov/libdl.so.2"\, O_RDONLY) = 3 close(3) = 0 open("/lib/i686/cmov/libm.so.6"\, O_RDONLY) = 3 close(3) = 0 open("/lib/i686/cmov/libcrypt.so.1"\, O_RDONLY) = 3 close(3) = 0 open("/lib/i686/cmov/libutil.so.1"\, O_RDONLY) = 3 close(3) = 0 open("/lib/i686/cmov/libpthread.so.0"\, O_RDONLY) = 3 close(3) = 0 open("/lib/i686/cmov/libc.so.6"\, O_RDONLY) = 3 close(3) = 0 open("/dev/urandom"\, O_RDONLY|O_LARGEFILE) = 3 close(3) = 0 open("/dev/null"\, O_RDONLY|O_LARGEFILE) = 3 close(3) = 0 close(1) = 0 open("lib/strict.pm"\, O_RDONLY|O_LARGEFILE) = 1 close(1) = 0

with file descriptor 0:

$ strace -eopen\,close ./perl -Ilib -e 'close STDIN; eval "use strict"; ' open("/etc/ld.so.cache"\, O_RDONLY) = 3 close(3) = 0 open("/lib/i686/cmov/libnsl.so.1"\, O_RDONLY) = 3 close(3) = 0 open("/lib/i686/cmov/libdl.so.2"\, O_RDONLY) = 3 close(3) = 0 open("/lib/i686/cmov/libm.so.6"\, O_RDONLY) = 3 close(3) = 0 open("/lib/i686/cmov/libcrypt.so.1"\, O_RDONLY) = 3 close(3) = 0 open("/lib/i686/cmov/libutil.so.1"\, O_RDONLY) = 3 close(3) = 0 open("/lib/i686/cmov/libpthread.so.0"\, O_RDONLY) = 3 close(3) = 0 open("/lib/i686/cmov/libc.so.6"\, O_RDONLY) = 3 close(3) = 0 open("/dev/urandom"\, O_RDONLY|O_LARGEFILE) = 3 close(3) = 0 open("/dev/null"\, O_RDONLY|O_LARGEFILE) = 3 close(3) = 0 close(0) = 0 open("lib/strict.pm"\, O_RDONLY|O_LARGEFILE) = 0

There's no final close(0) there.

I believe it's down to this piece of special case code in Perl_lex_next_chunk():

/* End of real input. Close filehandle (unless it was STDIN)\, * then add implicit termination. */ if ((PerlIO*)PL_parser->rsfp == PerlIO_stdin()) PerlIO_clearerr(PL_parser->rsfp); else if (PL_parser->rsfp) (void)PerlIO_close(PL_parser->rsfp);

which I guess should *ideally* be deciding to close the file handle based on whether it was opened for the parse\, not on whether it happens to be re-using the stdin slot. Which would mean storing state as to whether PL_parser->rsfp was "owned" by the parser struct (ie opened by the parser)\, or something passed in by an external owner.

The comment dates from commit f0e67a1d29102aa9. The previous version of the comment was:

/* Close the filehandle. Could be from * STDIN\, or a regular file. If we were reading code from * STDIN (because the commandline held no -e or filename) * then we don't close it\, we reset it so the code can * read from STDIN too. */

which (I think) agrees with my assessment - the intent of the check as given by the comment isn't the same as what the code actually tests.

Nicholas Clark

p5pRT commented 12 years ago

From @iabyn

On Mon\, Jan 09\, 2012 at 01:36:37PM +0000\, Nicholas Clark wrote:

On Sun\, Jan 08\, 2012 at 02:56:12PM +0000\, Dave Mitchell wrote:

It's not a bug.

I think it *is* a bug :-)

Ah\, so it is :-(. I'll mark the ticket as open again

-- Wesley Crusher gets beaten up by his classmates for being a smarmy git\, and consequently has a go at making some friends of his own age for a change. -- Things That Never Happen in "Star Trek" #18

p5pRT commented 12 years ago

@iabyn - Status changed from 'rejected' to 'open'

p5pRT commented 12 years ago

From @nwc10

On Fri Jan 13 06:03:24 2012\, davem wrote:

On Mon\, Jan 09\, 2012 at 01:36:37PM +0000\, Nicholas Clark wrote:

On Sun\, Jan 08\, 2012 at 02:56:12PM +0000\, Dave Mitchell wrote:

It's not a bug.

I think it *is* a bug :-)

Ah\, so it is :-(. I'll mark the ticket as open again

I've pushed a proposed fix to the branch smoke-me/RT37033

Nicholas Clark

p5pRT commented 12 years ago

From @xdg

On Sun\, Jan 8\, 2012 at 7:09 PM\, Ricardo SIGNES via RT \perlbug\-comment@perl\.org wrote:

tl;dr\, some edges are best left explained anew each time

I think the existing documentation could be stronger without getting into the details\, however.

perldata now:

...The filehandle is left open pointing to the line after __DATA__. It is the program's responsibility to C\ when it is done reading from it.

Perhaps that could be:

...The filehandle is left open pointing to the line after __DATA__. The program should C\ when it is done reading from it.

Leaving it open leaks filehandles if the module is reloaded for any reason\, so it's a safer practice to close it.

I'm not sure if we need to explain why since reloading is pretty rare\, but we could with a single sentence like that if we chose to.

-- David

p5pRT commented 12 years ago

From @nwc10

On Fri Feb 24 07:37:19 2012\, nicholas wrote:

On Fri Jan 13 06:03:24 2012\, davem wrote:

On Mon\, Jan 09\, 2012 at 01:36:37PM +0000\, Nicholas Clark wrote:

On Sun\, Jan 08\, 2012 at 02:56:12PM +0000\, Dave Mitchell wrote:

It's not a bug.

I think it *is* a bug :-)

Ah\, so it is :-(. I'll mark the ticket as open again

I've pushed a proposed fix to the branch smoke-me/RT37033

Merged to blead as a907031882152818

Pondering David's related suggested documentation tweak\, in the comments on this ticket. I think we should resolve that or move it to a new ticket before marking this ticket resolved.

Nicholas Clark

p5pRT commented 12 years ago

From @jkeenan

On Mon Feb 27 02:49:24 2012\, nicholas wrote:

Pondering David's related suggested documentation tweak\, in the comments on this ticket. I think we should resolve that or move it to a new ticket before marking this ticket resolved.

Nicholas Clark

In the interest of getting this ticket closed without opening a separate ticket for a documentation patch\, I have worked David Golden's suggestion into the patch attached.

Thank you very much. Jim Keenan

p5pRT commented 12 years ago

From @jkeenan

0001-Explain-why-program-must-close-__DATA__-handle.patch

```diff From 901a54241c6ff1423fc54301abddbf361c9139c3 Mon Sep 17 00:00:00 2001 From: jkeenan Date: Mon, 27 Feb 2012 12:34:10 -0500 Subject: [PATCH] Explain why program must close __DATA__ handle. Adapted from suggestion by David Golden++. For RT #37033. --- pod/perldata.pod | 18 +++++++++--------- 1 files changed, 9 insertions(+), 9 deletions(-) diff --git a/pod/perldata.pod b/pod/perldata.pod index 16ceb41..d14ac10 100644 --- a/pod/perldata.pod +++ b/pod/perldata.pod @@ -413,15 +413,15 @@ The two control characters ^D and ^Z, and the tokens __END__ and __DATA__ may be used to indicate the logical end of the script before the actual end of file. Any following text is ignored. -Text after __DATA__ may be read via the filehandle C, -where C is the package that was current when the __DATA__ -token was encountered. The filehandle is left open pointing to the -line after __DATA__. It is the program's responsibility to -C when it is done reading from it. For compatibility with -older scripts written before __DATA__ was introduced, __END__ behaves -like __DATA__ in the top level script (but not in files loaded with -C or C) and leaves the remaining contents of the -file accessible via C. +Text after __DATA__ may be read via the filehandle C, where +C is the package that was current when the __DATA__ token was +encountered. The filehandle is left open pointing to the line after __DATA__. +The program should C when it is done reading from it. (Leaving it +open leaks filehandles if the module is reloaded for any reason, so it's a +safer practice to close it.) For compatibility with older scripts written +before __DATA__ was introduced, __END__ behaves like __DATA__ in the top level +script (but not in files loaded with C or C) and leaves the +remaining contents of the file accessible via C. See L for more description of __DATA__, and an example of its use. Note that you cannot read from the DATA -- 1.6.3.2 ```

p5pRT commented 12 years ago

From @rjbs

Thanks\, applied as a6df44c751fa4d86cddb76cc0a040779fe2e7319 (unless revised for porting tests before pushing upstream).

p5pRT commented 12 years ago

From [Unknown Contact. See original ticket]

Thanks\, applied as a6df44c751fa4d86cddb76cc0a040779fe2e7319 (unless revised for porting tests before pushing upstream).

p5pRT commented 12 years ago

@rjbs - Status changed from 'open' to 'resolved'

Perl / perl5