binmode $fh, ':unix' loses track of the FD

[ap]

Description

Setting the :unix layer on a file handle after the fact using binmode() somehow decouples the file descriptor from the handle so that closing the handle doesn’t close the descriptor.

Steps to Reproduce

This loop (assuming you have an /etc/passwd and may read it) runs forever:

while () {
  open my $fh, '<:unix', '/etc/passwd' or die $!;
}

This loop terminates:

while () {
  open my $fh, '<', '/etc/passwd' or die $!;
  binmode $fh, ':unix';
}

After not all that many iterations it bails with the following error:

Too many open files at t.pl line 2.

Closing the handle explicitly doesn’t help. This still fails the same way:

while () {
  open my $fh, '<', '/etc/passwd' or die $!;
  binmode $fh, ':unix';
  close $fh;
}

What does work is going rogue and reaching down through the handle abstraction to close the descriptor itself. The following runs forever again:

require POSIX;
while () {
  open my $fh, '<', '/etc/passwd' or die $!;
  binmode $fh, ':unix';
  POSIX::close fileno $fh;
}

Expected behavior

All of the above examples should run forever.

Perl configuration

I am pasting the following just as an example since the issue template asks me to. In fact however I have tested this on any number of perls and it reproduces on all of them, going back to at least 5.8.9.

Summary of my perl5 (revision 5 version 30 subversion 3) configuration:

  Platform:
    osname=darwin
    osvers=21.0
    archname=darwin-thread-multi-2level
    uname='darwin smwk2.p1s.plx.sd.apple.com 21.0 darwin kernel version 20.1.0: wed nov 3 12:44:24 pdt 2021; root:xnu-7195.41.8.100.10~1development_x86_64 x86_64 '
    config_args='-ds -e -Dprefix=/usr -Dccflags=-g  -pipe  -Dldflags= -Dman3ext=3pm -Duseithreads -Duseshrplib -Dinc_version_list=none -Dcc=cc'
    hint=recommended
    useposix=true
    d_sigaction=define
    useithreads=define
    usemultiplicity=define
    use64bitint=define
    use64bitall=define
    uselongdouble=undef
    usemymalloc=n
    default_inc_excludes_dot=define
    bincompat5005=undef
  Compiler:
    cc='cc'
    ccflags =' -g -pipe -fno-strict-aliasing -fstack-protector-strong -DPERL_USE_SAFE_PUTENV'
    optimize='-Os'
    cppflags='-g -pipe -fno-strict-aliasing -fstack-protector-strong'
    ccversion=''
    gccversion='Apple LLVM 13.1.6 (clang-1316.0.20.6) [+internal-os, ptrauth-isa=deployment-target-based]'
    gccosandvers=''
    intsize=4
    longsize=8
    ptrsize=8
    doublesize=8
    byteorder=12345678
    doublekind=3
    d_longlong=define
    longlongsize=8
    d_longdbl=define
    longdblsize=16
    longdblkind=3
    ivtype='long'
    ivsize=8
    nvtype='double'
    nvsize=8
    Off_t='off_t'
    lseeksize=8
    alignbytes=8
    prototype=define
  Linker and Libraries:
    ld='cc'
    ldflags =' -fstack-protector-strong'
    libpth=/AppleInternal/Library/BuildRoots/66382bca-8bca-11ec-aade-6613bcf0e2ee/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX12.3.Internal.sdk/usr/local/lib /AppleInternal/Library/BuildRoots/66382bca-8bca-11ec-aade-6613bcf0e2ee/Applications/Xcode.app/Contents/Developer/Toolchains/OSX12.3.xctoolchain/usr/lib/clang/13.1.6/lib /AppleInternal/Library/BuildRoots/66382bca-8bca-11ec-aade-6613bcf0e2ee/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX12.3.Internal.sdk/usr/lib /AppleInternal/Library/BuildRoots/66382bca-8bca-11ec-aade-6613bcf0e2ee/Applications/Xcode.app/Contents/Developer/Toolchains/OSX12.3.xctoolchain/usr/lib /AppleInternal/Library/BuildRoots/66382bca-8bca-11ec-aade-6613bcf0e2ee/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/lib /usr/lib /usr/local/lib
    libs=
    perllibs=
    libc=
    so=dylib
    useshrplib=true
    libperl=libperl.dylib
    gnulibc_version=''
  Dynamic Linking:
    dlsrc=dl_dlopen.xs
    dlext=bundle
    d_dlsymun=undef
    ccdlflags=' '
    cccdlflags=' '
    lddlflags=' -bundle -undefined dynamic_lookup -fstack-protector-strong'


Characteristics of this binary (from libperl):
  Compile-time options:
    HAS_TIMES
    MULTIPLICITY
    PERLIO_LAYERS
    PERL_COPY_ON_WRITE
    PERL_DONT_CREATE_GVSV
    PERL_IMPLICIT_CONTEXT
    PERL_MALLOC_WRAP
    PERL_OP_PARENT
    PERL_PRESERVE_IVUV
    PERL_USE_SAFE_PUTENV
    USE_64_BIT_ALL
    USE_64_BIT_INT
    USE_ITHREADS
    USE_LARGE_FILES
    USE_LOCALE
    USE_LOCALE_COLLATE
    USE_LOCALE_CTYPE
    USE_LOCALE_NUMERIC
    USE_LOCALE_TIME
    USE_PERLIO
    USE_PERL_ATOF
    USE_REENTRANT_API
    USE_THREAD_SAFE_LOCALE
  Locally applied patches:
    /Library/Perl/Updates/<version> comes before system perl directories
    installprivlib and installarchlib points to the Updates directory
  Built under darwin
  Compiled at Feb 12 2022 01:42:24
  %ENV:
    PERL_CPANM_OPT="--no-man-pages --mirror-only --mirror /Users/ap/.local/share/minicpan"
  @INC:
    /Library/Perl/5.30/darwin-thread-multi-2level
    /Library/Perl/5.30
    /Network/Library/Perl/5.30/darwin-thread-multi-2level
    /Network/Library/Perl/5.30
    /Library/Perl/Updates/5.30.3
    /System/Library/Perl/5.30/darwin-thread-multi-2level
    /System/Library/Perl/5.30
    /System/Library/Perl/Extras/5.30/darwin-thread-multi-2level
    /System/Library/Perl/Extras/5.30

[Leont]

It gets confused because that binmode will result in :unix:perlio:unix, but the :unix layer isn't taking into account that there may be layers underneath it, so those don't end up being closed.

This shouldn't leak, but arguably it should also warn not to push a :unix on top of an existing IO stack.

[ap]

This shouldn't leak, but arguably it should also warn not to push a :unix on top of an existing IO stack.

Agreed. I hadn’t ever used binmode() like this before, so I didn’t realize that opening a file with some PerlIO layers is not the same as using binmode() on the filehandle with the same layers, that open() doesn’t set the given layers verbatim but has special logic to compute a useful layer stack from it, or that binmode() doesn’t even attempt to emulate that logic because all it does it push a layer verbatim. A warning would have clued me in that I didn’t know what I was doing.

[Grinnz]

The updated PerlIO docs may explain things a bit better: https://perldoc.perl.org/PerlIO#Layers

In a nutshell, there is a default layer stack defined by the system (generally, just :unix:crlf on windows and :unix:perlio otherwise) which can be overridden by the PERLIO env var or the ${^OPEN} var (the mechanism for open.pm);

specifying layers in open will use those layers instead of the defaults but apply the defaults under them if necessary (i.e. when specifying :encoding(UTF-8), you're really specifying :unix:perlio:encoding(UTF-8));

and binmode instead applies layers to the end of the current layer stack, some of which are not really layers but perform actions on the existing layer stack (like :raw or :pop).

[ap]

Sure – I’m well aware… now, that is. 🙂

What I’m saying is that a warning that :unix makes no sense to push on top of (if I understand correctly) any other layer would have tipped me off to check my understanding earlier.