Bug 977672: Assert O_NONBLOCK for watched file descriptors, r=kyle

We cannot use blocking file-descriptor I/O on the I/O thread. This
patch adds an assertion to UnixFdWatcher the tests for the O_NONBLOCK
flag, when installing a file descriptor. In UnixFileWatcher, the
Open method tests for the O_NONBLOCK flag for the opened file.

File-descriptor flags for UnixSocketImpl et al are currently set by
UnixSocketImpl itself. Later patches should move this into the
methods of connector classes.

ipc/unixfd/UnixFdWatcher.cpp

@@ -4,6 +4,7 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
+#include <fcntl.h>
 #include "UnixFdWatcher.h"
 
 #ifdef CHROMIUM_LOG
@@ -111,9 +112,17 @@ UnixFdWatcher::SetFd(int aFd)
 {
   MOZ_ASSERT(MessageLoopForIO::current() == mIOLoop);
   MOZ_ASSERT(!IsOpen());
+  MOZ_ASSERT(FdIsNonBlocking(aFd));
 
   mFd = aFd;
 }
 
+bool
+UnixFdWatcher::FdIsNonBlocking(int aFd)
+{
+  int flags = TEMP_FAILURE_RETRY(fcntl(aFd, F_GETFL));
+  return (flags > 0) && (flags & O_NONBLOCK);
+}
+
 }
 }

ipc/unixfd/UnixFdWatcher.h

@@ -52,6 +52,8 @@ protected:
   void SetFd(int aFd);
 
 private:
+  static bool FdIsNonBlocking(int aFd);
+
   MessageLoop* mIOLoop;
   ScopedClose mFd;
   MessageLoopForIO::FileDescriptorWatcher mReadWatcher;

ipc/unixfd/UnixFileWatcher.cpp

@@ -18,6 +18,7 @@ nsresult
 UnixFileWatcher::Open(const char* aFilename, int aFlags, mode_t aMode)
 {
   MOZ_ASSERT(MessageLoopForIO::current() == GetIOLoop());
+  MOZ_ASSERT(aFlags & O_NONBLOCK);
 
   int fd = TEMP_FAILURE_RETRY(open(aFilename, aFlags, aMode));
   if (fd < 0) {

ipc/unixfd/UnixSocketWatcher.cpp

@@ -29,25 +29,8 @@ UnixSocketWatcher::Connect(const struct sockaddr* aAddr, socklen_t aAddrLen)
   MOZ_ASSERT(IsOpen());
   MOZ_ASSERT(aAddr || !aAddrLen);
 
-  // Select non-blocking IO.
-  if (TEMP_FAILURE_RETRY(fcntl(GetFd(), F_SETFL, O_NONBLOCK)) < 0) {
-    OnError("fcntl", errno);
-    return NS_ERROR_FAILURE;
-  }
-
   if (connect(GetFd(), aAddr, aAddrLen) < 0) {
     if (errno == EINPROGRESS) {
-      // Select blocking IO again, since we've now at least queue'd the connect
-      // as nonblock.
-      int flags = TEMP_FAILURE_RETRY(fcntl(GetFd(), F_GETFL, 0));
-      if (flags < 0) {
-        OnError("fcntl", errno);
-        return NS_ERROR_FAILURE;
-      }
-      if (TEMP_FAILURE_RETRY(fcntl(GetFd(), F_SETFL, flags&~O_NONBLOCK)) < 0) {
-        OnError("fcntl", errno);
-        return NS_ERROR_FAILURE;
-      }
       mConnectionStatus = SOCKET_IS_CONNECTING;
       // Set up a write watch to receive the connect signal
       AddWatchers(WRITE_WATCHER, false);

ipc/unixsocket/UnixSocket.cpp

@@ -124,13 +124,6 @@ public:
    */
   void Listen();
 
-  /**
-   * Set up flags on whatever our current file descriptor is.
-   *
-   * @return true if successful, false otherwise
-   */
-  bool SetSocketFlags(int aFd);
-
   void GetSocketAddr(nsAString& aAddrStr)
   {
     if (!mConnector) {
@@ -156,6 +149,8 @@ public:
   void OnSocketCanSendWithoutBlocking() MOZ_OVERRIDE;
 
 private:
+  // Set up flags on whatever our current file descriptor is.
+  static bool SetSocketFlags(int aFd);
 
   void FireSocketError();
 
@@ -464,13 +459,12 @@ UnixSocketImpl::Listen()
       FireSocketError();
       return;
     }
-    SetFd(fd);
-
-    if (!SetSocketFlags(GetFd())) {
+    if (!SetSocketFlags(fd)) {
       NS_WARNING("Cannot set socket flags!");
       FireSocketError();
       return;
     }
+    SetFd(fd);
 
     // calls OnListening on success, or OnError otherwise
     nsresult rv = UnixSocketWatcher::Listen(
@@ -492,6 +486,11 @@ UnixSocketImpl::Connect()
       FireSocketError();
       return;
     }
+    if (!SetSocketFlags(fd)) {
+      NS_WARNING("Cannot set socket flags!");
+      FireSocketError();
+      return;
+    }
     SetFd(fd);
   }
 
@@ -512,16 +511,27 @@ UnixSocketImpl::SetSocketFlags(int aFd)
 {
   // Set socket addr to be reused even if kernel is still waiting to close
   int n = 1;
-  setsockopt(aFd, SOL_SOCKET, SO_REUSEADDR, &n, sizeof(n));
-
-  // Set close-on-exec bit.
-  int flags = fcntl(aFd, F_GETFD);
-  if (-1 == flags) {
+  if (setsockopt(aFd, SOL_SOCKET, SO_REUSEADDR, &n, sizeof(n)) < 0) {
     return false;
   }
 
+  // Set close-on-exec bit.
+  int flags = TEMP_FAILURE_RETRY(fcntl(aFd, F_GETFD));
+  if (-1 == flags) {
+    return false;
+  }
   flags |= FD_CLOEXEC;
-  if (-1 == fcntl(aFd, F_SETFD, flags)) {
+  if (-1 == TEMP_FAILURE_RETRY(fcntl(aFd, F_SETFD, flags))) {
+    return false;
+  }
+
+  // Set non-blocking status flag.
+  flags = TEMP_FAILURE_RETRY(fcntl(aFd, F_GETFL));
+  if (-1 == flags) {
+    return false;
+  }
+  flags |= O_NONBLOCK;
+  if (-1 == TEMP_FAILURE_RETRY(fcntl(aFd, F_SETFL, flags))) {
     return false;
   }
 
@@ -541,10 +551,10 @@ UnixSocketImpl::OnAccepted(int aFd)
 
   RemoveWatchers(READ_WATCHER|WRITE_WATCHER);
   Close();
-  SetSocket(aFd, SOCKET_IS_CONNECTED);
-  if (!SetSocketFlags(GetFd())) {
+  if (!SetSocketFlags(aFd)) {
     return;
   }
+  SetSocket(aFd, SOCKET_IS_CONNECTED);
 
   nsRefPtr<OnSocketEventTask> t =
     new OnSocketEventTask(this, OnSocketEventTask::CONNECT_SUCCESS);