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

Implicit filehandle close not working when STDERR closed #16209

Closed
p5pRT opened this issue Oct 25, 2017 · 10 comments
Closed

Implicit filehandle close not working when STDERR closed #16209

p5pRT opened this issue Oct 25, 2017 · 10 comments

Comments

@p5pRT
Copy link

p5pRT commented Oct 25, 2017

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

Searchable as RT132358$

@p5pRT
Copy link
Author

p5pRT commented Oct 25, 2017

From @steve-m-hay

Created by @steve-m-hay

I have recently noticed temporary files being left behind by
ExtUtils​::CBuilder's test suite, which I traced to the $SCRIPT
filehandle in ExtUtils​::CBuilder​::Platform​::Windows​::MSVC​::write_compiler_script().

It should be automatically closed when that function exits, but it
isn't - causing the attempted later deletion of it to fail because of
a familiar Windows problem of not being able to delete an open file.

Furthermore, I found that the problem weirdly only occurs with the
changes made by https://perl5.git.perl.org/perl.git/commit/581ae9ba65
in place.

The closure of STDERR is the key thing in that change, and the problem
is easily reproduced as follows​:

  use strict;
  use warnings;
  my $file = 'test.txt';
  close STDERR;
  {
  my $fh;
  if (open $fh, '>', $file) {
  print "Opened $file. Press <ENTER> to close and unlink it...\n";
  <STDIN>;
  }
  else {
  print "Open $file failed​: $!";
  exit;
  }
  }
  if (unlink $file) {
  print "Unlinked $file\n";
  }
  else {
  print "Unlink $file failed​: $!\n";
  }

On Windows, the $file does not get deleted at the end and the output is​:

  Opened test.txt. Press <ENTER> to close and unlink it...
  Unlink test.txt failed​: Permission denied

(Unfortunately, you probably won't see the problem as easily as that
on other OSes since they mostly don't have an issue deleting an open
file. But presumably the problem of $fh still being open should still
be evident by other means of inspection.)

If I simply comment out the "close STDERR;" line then the $file does
get deleted and the output is as expected​:

  Opened test.txt. Press <ENTER> to close and unlink it...
  Unlinked test.txt

Perl Info

Flags:
    category=core
    severity=high

Site configuration information for perl 5.27.6:

Configured by shay at Wed Oct 25 13:40:39 2017.

Summary of my perl5 (revision 5 version 27 subversion 6) configuration:
  Commit id: 0cdc775ef423ad6415e6f80b9244c17a52bf5149
  Platform:
    osname=MSWin32
    osvers=6.3
    archname=MSWin32-x86-multi-thread
    uname=''
    config_args='undef'
    hint=recommended
    useposix=true
    d_sigaction=undef
    useithreads=define
    usemultiplicity=define
    use64bitint=undef
    use64bitall=undef
    uselongdouble=undef
    usemymalloc=n
    default_inc_excludes_dot=define
    bincompat5005=undef
  Compiler:
    cc='cl'
    ccflags ='-nologo -GF -W3 -O1 -MD -Zi -DNDEBUG -GL -DWIN32
-D_CONSOLE -DNO_STRICT -D_CRT_SECURE_NO_DEPRECATE
-D_CRT_NONSTDC_NO_DEPRECATE  -DPERL_TEXTMODE_SCRIPTS
-DPERL_IMPLICIT_CONTEXT -DPERL_IMPLICIT_SYS'
    optimize='-O1 -MD -Zi -DNDEBUG -GL'
    cppflags='-DWIN32'
    ccversion='16.00.40219.01'
    gccversion=''
    gccosandvers=''
    intsize=4
    longsize=4
    ptrsize=4
    doublesize=8
    byteorder=1234
    doublekind=3
    d_longlong=undef
    longlongsize=8
    d_longdbl=define
    longdblsize=8
    longdblkind=0
    ivtype='long'
    ivsize=4
    nvtype='double'
    nvsize=8
    Off_t='__int64'
    lseeksize=8
    alignbytes=8
    prototype=define
  Linker and Libraries:
    ld='link'
    ldflags ='-nologo -nodefaultlib -debug -opt:ref,icf -ltcg
-libpath:"c:\perl\lib\CORE"  -machine:x86'
    libpth="C:\Program Files (x86)\Microsoft Visual Studio 10.0\VC\\lib"
    libs=oldnames.lib kernel32.lib user32.lib gdi32.lib winspool.lib
comdlg32.lib advapi32.lib shell32.lib ole32.lib oleaut32.lib
netapi32.lib uuid.lib ws2_32.lib mpr.lib winmm.lib    version.lib
odbc32.lib odbccp32.lib comctl32.lib msvcrt.lib
    perllibs=oldnames.lib kernel32.lib user32.lib gdi32.lib
winspool.lib  comdlg32.lib advapi32.lib shell32.lib ole32.lib
oleaut32.lib    netapi32.lib uuid.lib ws2_32.lib mpr.lib winmm.lib
version.lib odbc32.lib odbccp32.lib comctl32.lib msvcrt.lib
    libc=msvcrt.lib
    so=dll
    useshrplib=true
    libperl=perl527.lib
    gnulibc_version=''
  Dynamic Linking:
    dlsrc=dl_win32.xs
    dlext=dll
    d_dlsymun=undef
    ccdlflags=' '
    cccdlflags=' '
    lddlflags='-dll -nologo -nodefaultlib -debug -opt:ref,icf -ltcg
-libpath:"c:\perl\lib\CORE"  -machine:x86'



@INC for perl 5.27.6:
    C:/perl/site/lib
    C:/perl/lib


Environment for perl 5.27.6:
    HOME (unset)
    LANG (unset)
    LANGUAGE (unset)
    LD_LIBRARY_PATH (unset)
    LOGDIR (unset)
    PATH=C:\Windows\system32;C:\Windows;C:\Windows\system32\wbem;C:\perl\bin
    PERL_BADLANG (unset)
    SHELL (unset)

@p5pRT
Copy link
Author

p5pRT commented Oct 25, 2017

From @steve-m-hay

test.pl

@p5pRT
Copy link
Author

p5pRT commented Oct 25, 2017

From zefram@fysh.org

Steve Hay wrote​:

The closure of STDERR is the key thing in that change,

Since you didn't open a new standard error stream deliberately, the next
thing you open gets that file descriptor, becoming the process's new
standard error stream (even though not mapped as STDERR). perl quite
sensibly declines to implicitly close this. Rather than just close
STDERR, you should instead reopen it pointing to something innocuous.
Or if you really want STDERR closed, you should at least point the file
descriptor to something innocuous.

-zefram

@p5pRT
Copy link
Author

p5pRT commented Oct 25, 2017

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

@p5pRT
Copy link
Author

p5pRT commented Oct 25, 2017

From @cpansprout

On Wed, 25 Oct 2017 07​:25​:36 -0700, zefram@​fysh.org wrote​:

Steve Hay wrote​:

The closure of STDERR is the key thing in that change,

Since you didn't open a new standard error stream deliberately, the next
thing you open gets that file descriptor, becoming the process's new
standard error stream (even though not mapped as STDERR). perl quite
sensibly declines to implicitly close this. Rather than just close
STDERR, you should instead reopen it pointing to something innocuous.
Or if you really want STDERR closed, you should at least point the file
descriptor to something innocuous.

Or just close $fh explicitly.

--

Father Chrysostomos

@p5pRT
Copy link
Author

p5pRT commented Oct 25, 2017

From @steve-m-hay

On Wed, 25 Oct 2017 07​:25​:36 -0700, zefram@​fysh.org wrote​:

Steve Hay wrote​:

The closure of STDERR is the key thing in that change,

Since you didn't open a new standard error stream deliberately, the next
thing you open gets that file descriptor, becoming the process's new
standard error stream (even though not mapped as STDERR). perl quite
sensibly declines to implicitly close this. Rather than just close
STDERR, you should instead reopen it pointing to something innocuous.
Or if you really want STDERR closed, you should at least point the file
descriptor to something innocuous.

-zefram

Thanks. I should have thought of that. Now fixed in https://perl5.git.perl.org/perl.git/commit/b2b30beea3

@p5pRT
Copy link
Author

p5pRT commented Oct 25, 2017

From @steve-m-hay

On Wed, 25 Oct 2017 08​:45​:45 -0700, sprout wrote​:

On Wed, 25 Oct 2017 07​:25​:36 -0700, zefram@​fysh.org wrote​:

Steve Hay wrote​:

The closure of STDERR is the key thing in that change,

Since you didn't open a new standard error stream deliberately, the next
thing you open gets that file descriptor, becoming the process's new
standard error stream (even though not mapped as STDERR). perl quite
sensibly declines to implicitly close this. Rather than just close
STDERR, you should instead reopen it pointing to something innocuous.
Or if you really want STDERR closed, you should at least point the file
descriptor to something innocuous.

Or just close $fh explicitly.

Yes, that also works, but it's a shame that code such as that in write_compiler_script() can be affected from a distance like this. I've never considered it before, but clearly you can't rely on implicit close always working​: due to possible interference from afar you must always explicitly close() if you really need the file to be closed.

I'm wondering if a warning somewhere in the system might have helped here? Either when opening the file and finding that the stderr file descriptor has been used for it, or when refraining from doing the implicit close on that stream? Neither thing was what was expected/wanted in this case, but warnings would be annoying in other cases where these actions were intended.

@p5pRT
Copy link
Author

p5pRT commented Oct 25, 2017

From @exodist

Would be nice to have some control over file descriptors used in open. For
instance I have code that intentionally closes STDOUT and STDERR and opens
fh's with the intention of getting the file descriptors 1 and 2. I have to
make sure to close/open in the correct order and rely on the side-effect
that they get the recently closed numbers 1 and 2. Would be nice to be able
to tell open that I want to get a specific descriptor and have it fail if
it is already used.

Would also be nice to be able to tell open not to use any "special"
descriptors like 0 1 and 2.

-Chad

On Wed, Oct 25, 2017 at 9​:59 AM, Steve Hay via RT <perlbug-followup@​perl.org

wrote​:

On Wed, 25 Oct 2017 08​:45​:45 -0700, sprout wrote​:

On Wed, 25 Oct 2017 07​:25​:36 -0700, zefram@​fysh.org wrote​:

Steve Hay wrote​:

The closure of STDERR is the key thing in that change,

Since you didn't open a new standard error stream deliberately, the
next
thing you open gets that file descriptor, becoming the process's new
standard error stream (even though not mapped as STDERR). perl quite
sensibly declines to implicitly close this. Rather than just close
STDERR, you should instead reopen it pointing to something innocuous.
Or if you really want STDERR closed, you should at least point the file
descriptor to something innocuous.

Or just close $fh explicitly.

Yes, that also works, but it's a shame that code such as that in
write_compiler_script() can be affected from a distance like this. I've
never considered it before, but clearly you can't rely on implicit close
always working​: due to possible interference from afar you must always
explicitly close() if you really need the file to be closed.

I'm wondering if a warning somewhere in the system might have helped here?
Either when opening the file and finding that the stderr file descriptor
has been used for it, or when refraining from doing the implicit close on
that stream? Neither thing was what was expected/wanted in this case, but
warnings would be annoying in other cases where these actions were intended.

---
via perlbug​: queue​: perl5 status​: open
https://rt-archive.perl.org/perl5/Ticket/Display.html?id=132358

@p5pRT
Copy link
Author

p5pRT commented Oct 25, 2017

From @Leont

On Wed, Oct 25, 2017 at 7​:54 PM, Chad Granum <exodist7@​gmail.com> wrote​:

Would be nice to have some control over file descriptors used in open.
For instance I have code that intentionally closes STDOUT and STDERR and
opens fh's with the intention of getting the file descriptors 1 and 2. I
have to make sure to close/open in the correct order and rely on the
side-effect that they get the recently closed numbers 1 and 2. Would be
nice to be able to tell open that I want to get a specific descriptor and
have it fail if it is already used.

Would also be nice to be able to tell open not to use any "special"
descriptors like 0 1 and 2.

perl -e 'open STDOUT, ">", "foo.txt" or die "Couldnt open​: $!"; warn fileno
STDOUT'

Leon

@p5pRT
Copy link
Author

p5pRT commented Oct 26, 2017

@steve-m-hay - Status changed from 'open' to 'resolved'

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant