[PATCH] fclose: avoid double close race when possible

[Eric Blake]

Calling close(fileno(fp)) prior to fclose(fp) is racy in a
multi-threaded application - some other thread could open a new file,
which is then inadvertently closed by the fclose that we thought
should fail with EBADF.  For mingw, this is no worse than the race
already present in close_fd_maybe_socket for calling closesocket()
prior to _close(), but for all other platforms, we might as well be
nice and avoid the race.

* lib/fclose.c (rpl_fclose): Rewrite to avoid double-close race on
all but WINDOWS_SOCKETS.

Signed-off-by: Eric Blake <address@hidden>
---

This patch is necessary to avoid a regression on libvirt on Linux.

Is it worth thinking about replacing _every_ standard function on
mingw that can possibly create an fd in another thread (obviously,
as an optional module, only needed in multithreaded gnulib clients),
by grabbing a mutex to thus guarantee atomicity around otherwise
racy operations like the one in our fclose() wrapper?  Or do we
just chalk up the race to the poor quality of mingw in the first
place, since more compliant platforms don't have the race, and since
the cost of replacing everything just to guarantee atomicity sounds
like it would significantly slow down mingw execution.

Or do we come up with some alternative on mingw that avoids the
race?  Perhaps opening a temporary fd on /dev/null, then using
dup2 to overwrite the fd underlying the file pointer after the
fflush(), so that we can then use fclose() and expect success
(since fclose is no longer operating on a socket fd) while still
gracefully closing out the socket?  For that matter, does dup2()
on mingw properly handle the case where the destination fd is
an open socket?

 ChangeLog    |    6 ++++++
 lib/fclose.c |   22 +++++++++++++++++++---
 2 files changed, 25 insertions(+), 3 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 74919bb..c448183 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,9 @@
+2011-05-10  Eric Blake  <address@hidden>
+
+       fclose: avoid double close race when possible
+       * lib/fclose.c (rpl_fclose): Rewrite to avoid double-close race on
+       all but WINDOWS_SOCKETS.
+
 2011-05-10  Bastien Roucariès  <address@hidden>

        openat: correct new comment
diff --git a/lib/fclose.c b/lib/fclose.c
index bed561b..018724b 100644
--- a/lib/fclose.c
+++ b/lib/fclose.c
@@ -32,6 +32,7 @@ rpl_fclose (FILE *fp)
 {
   int saved_errno = 0;
   int fd;
+  int result = 0;

   /* Don't change behavior on memstreams.  */
   fd = fileno (fp);
@@ -45,15 +46,30 @@ rpl_fclose (FILE *fp)
       && fflush (fp))
     saved_errno = errno;

+#if WINDOWS_SOCKETS
+  /* There is a minor race where some other thread could open fd
+     between our close and fopen, but it is no worse than the race in
+     close_fd_maybe_socket.  */
   if (close (fd) < 0 && saved_errno == 0)
     saved_errno = errno;

-  fclose (fp); /* will fail with errno = EBADF */
+  fclose (fp); /* will fail with errno = EBADF, if we did not lose a race */

   if (saved_errno != 0)
     {
       errno = saved_errno;
-      return EOF;
+      result = EOF;
     }
-  return 0;
+
+#else /* !WINDOWS_SOCKETS */
+  /* No race here.  */
+  result = fclose (fp);
+
+# if REPLACE_FCHDIR
+  if (result == 0)
+    unregister_shadow_fd (fd);
+# endif
+#endif /* !WINDOWS_SOCKETS */
+
+  return result;
 }
-- 
1.7.4.4