Bug 977672: Assert O_NONBLOCK for watched file descriptors, r=kyle
authorThomas Zimmermann <tdz@users.sourceforge.net>
Fri, 28 Feb 2014 10:16:52 +0100
changeset 189414 4f7dc3feaf3178dd241d6b90543fc83196639f12
parent 189397 8a80d7a4e4db0f67bca8587a8d78bdc4dbbfc220
child 189415 196d98e2fc8ade808ee045e93a652135f400833b
push id474
push userasasaki@mozilla.com
push dateMon, 02 Jun 2014 21:01:02 +0000
treeherdermozilla-release@967f4cf1b31c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerskyle
bugs977672
milestone30.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
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
ipc/unixfd/UnixFdWatcher.h
ipc/unixfd/UnixFileWatcher.cpp
ipc/unixfd/UnixSocketWatcher.cpp
ipc/unixsocket/UnixSocket.cpp
--- a/ipc/unixfd/UnixFdWatcher.cpp
+++ b/ipc/unixfd/UnixFdWatcher.cpp
@@ -1,14 +1,15 @@
 /* -*- Mode: c++; c-basic-offset: 2; indent-tabs-mode: nil; tab-width: 40 -*- */
 /* vim: set ts=2 et sw=2 tw=80: */
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * 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
 #undef CHROMIUM_LOG
 #endif
 
 #if defined(MOZ_WIDGET_GONK)
 #include <android/log.h>
@@ -106,14 +107,22 @@ UnixFdWatcher::UnixFdWatcher(MessageLoop
   MOZ_ASSERT(mIOLoop);
 }
 
 void
 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);
+}
+
 }
 }
--- a/ipc/unixfd/UnixFdWatcher.h
+++ b/ipc/unixfd/UnixFdWatcher.h
@@ -47,16 +47,18 @@ public:
   virtual void OnError(const char* aFunction, int aErrno);
 
 protected:
   UnixFdWatcher(MessageLoop* aIOLoop);
   UnixFdWatcher(MessageLoop* aIOLoop, int aFd);
   void SetFd(int aFd);
 
 private:
+  static bool FdIsNonBlocking(int aFd);
+
   MessageLoop* mIOLoop;
   ScopedClose mFd;
   MessageLoopForIO::FileDescriptorWatcher mReadWatcher;
   MessageLoopForIO::FileDescriptorWatcher mWriteWatcher;
 };
 
 }
 }
--- a/ipc/unixfd/UnixFileWatcher.cpp
+++ b/ipc/unixfd/UnixFileWatcher.cpp
@@ -13,16 +13,17 @@ namespace ipc {
 UnixFileWatcher::~UnixFileWatcher()
 {
 }
 
 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) {
     OnError("open", errno);
     return NS_ERROR_FAILURE;
   }
   SetFd(fd);
   OnOpened();
--- a/ipc/unixfd/UnixSocketWatcher.cpp
+++ b/ipc/unixfd/UnixSocketWatcher.cpp
@@ -24,35 +24,18 @@ void UnixSocketWatcher::Close()
 
 nsresult
 UnixSocketWatcher::Connect(const struct sockaddr* aAddr, socklen_t aAddrLen)
 {
   MOZ_ASSERT(MessageLoopForIO::current() == GetIOLoop());
   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);
     } else {
       OnError("connect", errno);
     }
     return NS_ERROR_FAILURE;
   }
--- a/ipc/unixsocket/UnixSocket.cpp
+++ b/ipc/unixsocket/UnixSocket.cpp
@@ -119,23 +119,16 @@ public:
    */
   void Connect();
 
   /**
    * Run bind/listen to prepare for further runs of accept()
    */
   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) {
       NS_WARNING("No connector to get socket address from!");
       aAddrStr.Truncate();
       return;
     }
     mConnector->GetSocketAddr(mAddr, aAddrStr);
@@ -151,16 +144,18 @@ public:
   void OnAccepted(int aFd) MOZ_OVERRIDE;
   void OnConnected() MOZ_OVERRIDE;
   void OnError(const char* aFunction, int aErrno) MOZ_OVERRIDE;
   void OnListening() MOZ_OVERRIDE;
   void OnSocketCanReceiveWithoutBlocking() MOZ_OVERRIDE;
   void OnSocketCanSendWithoutBlocking() MOZ_OVERRIDE;
 
 private:
+  // Set up flags on whatever our current file descriptor is.
+  static bool SetSocketFlags(int aFd);
 
   void FireSocketError();
 
   /**
    * Raw data queue. Must be pushed/popped from IO thread only.
    */
   typedef nsTArray<UnixSocketRawData* > UnixSocketRawDataQueue;
   UnixSocketRawDataQueue mOutgoingQ;
@@ -459,23 +454,22 @@ UnixSocketImpl::Listen()
 
   if (!IsOpen()) {
     int fd = mConnector->Create();
     if (fd < 0) {
       NS_WARNING("Cannot create socket fd!");
       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(
       reinterpret_cast<struct sockaddr*>(&mAddr), mAddrSize);
     NS_WARN_IF(NS_FAILED(rv));
   }
 }
 
@@ -487,16 +481,21 @@ UnixSocketImpl::Connect()
 
   if (!IsOpen()) {
     int fd = mConnector->Create();
     if (fd < 0) {
       NS_WARNING("Cannot create socket fd!");
       FireSocketError();
       return;
     }
+    if (!SetSocketFlags(fd)) {
+      NS_WARNING("Cannot set socket flags!");
+      FireSocketError();
+      return;
+    }
     SetFd(fd);
   }
 
   if (!mConnector->CreateAddr(false, mAddrSize, mAddr, mAddress.get())) {
     NS_WARNING("Cannot create socket address!");
     FireSocketError();
     return;
   }
@@ -507,26 +506,37 @@ UnixSocketImpl::Connect()
   NS_WARN_IF(NS_FAILED(rv));
 }
 
 bool
 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));
+  if (setsockopt(aFd, SOL_SOCKET, SO_REUSEADDR, &n, sizeof(n)) < 0) {
+    return false;
+  }
 
   // Set close-on-exec bit.
-  int flags = fcntl(aFd, F_GETFD);
+  int flags = TEMP_FAILURE_RETRY(fcntl(aFd, F_GETFD));
   if (-1 == flags) {
     return false;
   }
+  flags |= FD_CLOEXEC;
+  if (-1 == TEMP_FAILURE_RETRY(fcntl(aFd, F_SETFD, flags))) {
+    return false;
+  }
 
-  flags |= FD_CLOEXEC;
-  if (-1 == fcntl(aFd, F_SETFD, flags)) {
+  // 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;
   }
 
   return true;
 }
 
 void
 UnixSocketImpl::OnAccepted(int aFd)
@@ -536,20 +546,20 @@ UnixSocketImpl::OnAccepted(int aFd)
 
   if (!mConnector->SetUp(aFd)) {
     NS_WARNING("Could not set up socket!");
     return;
   }
 
   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);
   NS_DispatchToMainThread(t);
 
   AddWatchers(READ_WATCHER, true);
   if (!mOutgoingQ.IsEmpty()) {
     AddWatchers(WRITE_WATCHER, false);