make EINTR/EINPROGRESS real errors

They should never happen on Linux, but if they do then this implies that
whatever guarantees would normally have been obtained by a successful
close have gone out the window, which is a genuine error.

(This mostly matters on network filesystems)

Author: mvduin

src/abstract_socket.cc

@@ -121,29 +121,43 @@ NAN_METHOD(Close) {
     assert(info.Length() == 1);
     fd = info[0]->Int32Value();
 
-    // Suppress EINTR and EINPROGRESS.  EINTR means that the close() system call
-    // was interrupted by a signal.  According to POSIX, the file descriptor is
-    // in an undefined state afterwards.  It's not safe to try closing it again
-    // because it may have been closed, despite the signal.  If we call close()
-    // again, then it would either:
+    // POSIX 2008 states that it unspecified what the state of a file descriptor
+    // is if close() is interrupted by a signal and fails with EINTR.  This is a
+    // problem for multi-threaded programs since if the fd was actually closed
+    // it may already be reused by another thread hence it is unsafe to attempt
+    // to close it again.  In 2012, POSIX approved a clarification that aimed
+    // to deal with this mess:  http://austingroupbugs.net/view.php?id=529#c1200
     //
-    //   a) fail with EBADF, or
+    // The short summary is that if the fd is never valid after close(), as is
+    // the case in Linux, then <unistd.h> should add:
     //
-    //   b) close the wrong file descriptor if another thread or a signal handler
-    //      has reused it in the mean time.
+    //      #define POSIX_CLOSE_RESTART 0
     //
-    // Neither is what we want but scenario B is particularly bad.  Not retrying
-    // the close() could, in theory, lead to file descriptor leaks but, in
-    // practice, operating systems do the right thing and close the file
-    // descriptor, regardless of whether the operation was interrupted by
-    // a signal.
+    // and the new posix_close() should be implemented as something like:
     //
-    // EINPROGRESS is benign.  It means the close operation was interrupted but
-    // that the file descriptor has been closed or is being closed in the
-    // background.  It's informative, not an error.
+    //      int posix_close(int fd, int flags) {
+    //          int r = close(fd);
+    //          if (r < 0 && errno == EINTR)
+    //              return 0 or set errno to EINPROGRESS;
+    //          return r;
+    //      }
+    //
+    // In contrast, on systems where EINTR means the close() didn't happen (like
+    // HP-UX), POSIX_CLOSE_RESTART should be non-zero and if passed as flag to
+    // posix_close() it should automatically retry close() on EINTR.
+    //
+    // Of course this is nice and all, but apparently adding one constant and
+    // a trivial wrapper was way too much effort for the glibc project:
+    //      https://sourceware.org/bugzilla/show_bug.cgi?id=16302
+    //
+    // But since we actually only care about Linux, we don't have to worry about
+    // all this anyway.  close() always means the fd is gone, even if an error
+    // occurred.  This elevates EINTR to the status of real error, since it
+    // implies behaviour associated with close (e.g. flush) was aborted and can
+    // not be retried since the fd is gone.
+
     err = 0;
     if (close(fd))
-      if (errno != EINTR && errno != EINPROGRESS)
         err = -errno;
 
     info.GetReturnValue().Set(err);