fclose: avoid double close race when possible

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 <eblake@redhat.com>

diff --git a/ChangeLog b/ChangeLog
index 74919bb30ceebe344ad3f83b4e23e5f1f7b9901f..c4481838b34ea1192444e68feb3b21a7355626d6 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,9 @@
+2011-05-10  Eric Blake  <eblake@redhat.com>
+
+       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  <roucaries.bastien@gmail.com>
 
        openat: correct new comment

diff --git a/lib/fclose.c b/lib/fclose.c
index bed561bdb1d57400c9507635681a1e3869d52912..018724b4023eaf81ed8988219bb6757d48464672 100644 (file)
--- 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;
 }