Re: fdopendir closes the file descriptor on MinGW

[Eli Zaretskii]

[Please CC me on any replies, as I'm not subscribed to the list.]

> Date: Sun, 15 Mar 2015 20:10:30 -0700
> From: David Grayson <address@hidden>
> 
> Hello.  When running under MinGW on Windows, there seems to be a bug
> in Gnulib's fdopendir implementation.  Gnulib's fdopendir closes the
> file descriptor that was passed to it as an argument.  It then tries
> to reopen the directory using the same file descriptor, but that
> doesn't seem to work in MinGW, and so the file descriptor remains
> closed after fdopendir returns.

My analysis of the code is similar, see below.

> Here is an example of some code that exhibits the bug:
> 
>   int fd = open("emptydir", O_RDONLY | O_DIRECTORY | O_NOCTTY | O_NONBLOCK);
>   printf("dup(fd) = %d\n", dup(fd));
>   fdopendir(fd);
>   printf("dup(fd) = %d\n", dup(fd));
> 
> Under MinGW, the second call to dup will fail and return -1 because
> fdopendir closes the file descriptor.

Indeed, that is what I see as well.

> Ultimately, I am trying to compile Grep 2.21 in Windows with MinGW so
> that I can have a good tool for searching files.  (I can't find any
> recent version of Grep compiled for Windows which doesn't have extra
> dependencies.)

Side note: what's wrong with this port:

  
http://sourceforge.net/projects/ezwinports/files/grep-2.10-w32-bin.zip/download

I see no important changes since 2.10 in NEWS, FWIW.  (Granted, I'm
using that port myself all the time.)

And if by "extra dependencies" you mean external libraries, then the
above binary zip includes all the dependency DLLs in the same zip
archive, so you just need to unzip it.

> The main source file (main.c) is attached to this message, and you can
> also read it here:
> 
>   http://www.davidegrayson.com/keep/gnulib_150315/main.c
> 
> When I run the code in Arch Linux, it gives the expected output:
> 
>   $ ./src/testgnulib
>   dup(fd) = 4
>   dup(fd) = 5
>   emptydir
>   emptydir

Are you sure Gnulib's fdopendir is at all used on GNU/Linux systems?
Don't they have fdopendir in their libc?  If so, the comparison
doesn't really say anything about Gnulib's fdopendir.

Anyway, after the simple change suggested below, I see the same output
on MinGW as you report for GNU/Linux.

> When I run the code in Windows 8.1 64-bit, it gives bad results.  I
> compiled the code with MinGW (gcc.exe (i686-posix-dwarf-rev1, Built by
> MinGW-W64 project) 4.9.2) and a patched version of make (3.82-pololu2
> from https://github.com/pololu/make).  Most of the other utilities on
> my PATH in Windows come from Msysgit.  I ran "./configure" in Git
> Bash.  Then I invoked "make" with the following command, because
> CreateProcess does not recognize paths like /c/git/bin/mkdir:
> 
>   make MKDIR_P='mkdir -p' SED=sed

Another aside: I think you are making your life much harder by using
these kludges.  The recommended way of building GNU software with
MinGW is to use the MSYS Bash and tools, including MSYS Make.  (It is
true that Msysgit comes with some of the MSYS environment, but since
it is primarily meant for running Git, it isn't necessarily suited for
configuring and building MinGW ports; in particular, some of the
programs in Msysgit are actually MinGW programs, not MSYS programs,
and so cannot grok the /c/foo/bar style of file names.  Other
programs, like "make", are simply missing from Msysgit.)

> If I comment out the "close (fd);" line in fdopendir.c, then the
> program behaves as expected under MinGW, returning the same output
> that it did in Arch Linux.

But that's not the right fix, of course, as the comments and the
design of Gnulib's fdopendir clearly show.

> Neither the POSIX documentation nor the Gnulib documentation for
> fdopendir mentions that the file descriptor might get closed by
> fdopendir, so it seems like a bug for that to happen.

It is indeed a bug.

> The POSIX documentation for fdopendir says that when closedir() is
> called on the returned pointer, the file descriptor (fd) shall be
> closed.  That means that somehow, some information about fd has to be
> associated with the returned DIR pointer.

Yes.  Gnulib maintains the correspondence between the DIR stream and
the file descriptor in its internal data structure.  See the source
for Gnulib's 'open' for some insight.

> I looked at the source code of Gnulib's fdopendir and tried to figure
> out what is going on, but I can't say I totally understand it.
> Gnulib's fdopendir first closes the passed file descriptor, and then
> it uses a non-thread-safe strategy involving duplication and recursion
> in order to reopen the directory in a way so that it happens to reuse
> the same file descriptor number.  This works on Linux but apparently
> does not work on MinGW.

Correct.  And the reason for that is very simple: Gnulib's fdopendir
never reopens the original file descriptor when it succeeds to open a
directory!

> This is speculation, but I suspect that this dup/recursive strategy in
> fdopendir is there in order to make sure that the returned DIR pointer
> is using the right file descriptor, so that closedir will end up
> closing the file descriptor as POSIX requires.  But on MinGW+Gnulib,
> dirfd(DIR *) returns -1, so it makes me think that DIR pointers don't
> actually contain file descriptors or close them when closedir is
> called; they might use some kind of native Win32 HANDLE instead.

DIR streams indeed don't have an underlying Posix-style file
descriptor on Windows, because the Posix-style 'open' doesn't work on
directories on Windows.  Gnulib makes it "work" by opening the null
device instead, just so the caller has some valid file descriptor to
play with.  Gnulib then arranges for all the functions that might use
the descriptor to actually access the directory to be overridden by
Gnulib implementations.  These implementations use the directory name
(stashed away by 'open') instead of the (useless) file descriptor.
One example of this is fdopendir itself: you will see in the source
that it retrieves the directory name by calling _gl_directory_name,
and then calls opendir with that name.

> To make fdopendir be POSIX compliant on MinGW, it seems like you would
> want to somehow associate the fd with the DIR pointer even if the fd
> isn't used by typical dirent functions.  Then when closedir is called,
> you would want to retrieve that fd and close it, as a side effect.

The logic of fdopendir's implementation is simple, and is outlined in
the comments to fdopendir_with_dup.  It is based on the traditional
behavior of 'dup' and 'dup2', whereby they return the smallest file
descriptor that is not in use; the MS-Windows runtime behaves the
same.  Here's the comment in fdopendir_with_dup which describes the
main idea:

   This function makes sure that FD is closed and all file descriptors
   less than FD are open, and then calls fd_clone_opendir on a dup of
   FD.  That way, barring race conditions, fd_clone_opendir returns a
   stream whose file descriptor is FD.

This obviously assumes that fd_clone_opendir will ask the OS for a new
file descriptor, and since all the descriptors below FD are in use,
that new file descriptor will be exactly FD.

However, since no such descriptor allocation happens on Windows, the
code should do that itself instead.  A simple patch to fdopendir.c
which fixes the problem for me is presented below.  I conditioned it
on MS-Windows, because I don't know which other platforms will need
that 'dup' call; perhaps Paul or Jim will know better.

An alternative would be to use Gnulib's opendir instead, which seems
to already take care of allocating a file descriptor.  However,
Gnulib's opendir is not being linked into the test program; instead,
MinGW's own opendir is.  Moreover, I don't see how Gnulib's dirfd
could work for MinGW, but perhaps I'm missing something.

--- lib/fdopendir.c~0   2015-03-15 22:24:30 +0200
+++ lib/fdopendir.c     2015-03-18 08:06:54 +0200
@@ -37,6 +37,12 @@
 #  define REPLACE_FCHDIR 0
 # endif
 
+# if (defined _WIN32 || defined __WIN32__) && ! defined __CYGWIN__
+#  define DUP_FD 1
+# else
+#  define DUP_FD 0
+# endif
+
 static DIR *fdopendir_with_dup (int, int, struct saved_cwd const *);
 static DIR *fd_clone_opendir (int, struct saved_cwd const *);
 
@@ -118,7 +124,7 @@ fdopendir_with_dup (int fd, int older_du
           close (fd);
           dir = fd_clone_opendir (dupfd, cwd);
           saved_errno = errno;
-          if (! dir)
+          if (! dir || DUP_FD)
             {
               int fd1 = dup (dupfd);
               if (fd1 != fd)