Fix spurious socket notifications on OS X and iOS

Core Foundation Framework forwards notifications about socket activity
through a callback function which called from the run loop.

The default behavior of Core Foundation is to automatically re-enable the
read callback after each notification, and we explicitly enabled the same
behavior for the write callback.

With this behavior, if the client did multiple recv() calls in response to
the first notification in a series of read notifications, the client would
still get the QSocketNotifier notifications for the data that was already
read.

To get rid of these extra notifications, we disable automatically re-enabling
the callbacks, and then manually enable them on each run loop pass.

Task-number: QTBUG-48556
Change-Id: I0b060222b787f45600be0cb7da85d04aef415e57
Reviewed-by: Tor Arne Vestbø <tor.arne.vestbo@theqtcompany.com>

src/platformsupport/cfsocketnotifier/qcfsocketnotifier.cpp

@@ -55,11 +55,15 @@ void qt_mac_socket_callback(CFSocketRef s, CFSocketCallBackType callbackType, CF
     // notifier is now gone. The upshot is we have to check the notifier
     // every time.
     if (callbackType == kCFSocketReadCallBack) {
-        if (socketInfo->readNotifier)
+        if (socketInfo->readNotifier && socketInfo->readEnabled) {
+            socketInfo->readEnabled = false;
             QGuiApplication::sendEvent(socketInfo->readNotifier, &notifierEvent);
+        }
     } else if (callbackType == kCFSocketWriteCallBack) {
-        if (socketInfo->writeNotifier)
+        if (socketInfo->writeNotifier && socketInfo->writeEnabled) {
+            socketInfo->writeEnabled = false;
             QGuiApplication::sendEvent(socketInfo->writeNotifier, &notifierEvent);
+        }
     }
 
     if (cfSocketNotifier->maybeCancelWaitForMoreEvents)
@@ -88,12 +92,12 @@ void qt_mac_remove_socket_from_runloop(const CFSocketRef socket, CFRunLoopSource
     CFRunLoopRemoveSource(CFRunLoopGetMain(), runloop, kCFRunLoopCommonModes);
     CFSocketDisableCallBacks(socket, kCFSocketReadCallBack);
     CFSocketDisableCallBacks(socket, kCFSocketWriteCallBack);
-    CFRunLoopSourceInvalidate(runloop);
 }
 
 QCFSocketNotifier::QCFSocketNotifier()
-:eventDispatcher(0)
-, maybeCancelWaitForMoreEvents(0)
+    : eventDispatcher(0)
+    , maybeCancelWaitForMoreEvents(0)
+    , enableNotifiersObserver(0)
 {
 
 }
@@ -150,36 +154,34 @@ void QCFSocketNotifier::registerSocketNotifier(QSocketNotifier *notifier)
         }
 
         CFOptionFlags flags = CFSocketGetSocketFlags(socketInfo->socket);
-        flags |= kCFSocketAutomaticallyReenableWriteCallBack; //QSocketNotifier stays enabled after a write
-        flags &= ~kCFSocketCloseOnInvalidate; //QSocketNotifier doesn't close the socket upon destruction/invalidation
+        // QSocketNotifier doesn't close the socket upon destruction/invalidation
+        flags &= ~kCFSocketCloseOnInvalidate;
+        // Expicitly disable automatic re-enable, as we do that manually on each runloop pass
+        flags &= ~(kCFSocketAutomaticallyReenableWriteCallBack | kCFSocketAutomaticallyReenableReadCallBack);
         CFSocketSetSocketFlags(socketInfo->socket, flags);
 
-        // Add CFSocket to runloop.
-        if (!(socketInfo->runloop = qt_mac_add_socket_to_runloop(socketInfo->socket))) {
-            qWarning("QEventDispatcherMac::registerSocketNotifier: Failed to add CFSocket to runloop");
-            CFSocketInvalidate(socketInfo->socket);
-            CFRelease(socketInfo->socket);
-            return;
-        }
-
-        // Disable both callback types by default. This must be done after
-        // we add the CFSocket to the runloop, or else these calls will have
-        // no effect.
-        CFSocketDisableCallBacks(socketInfo->socket, kCFSocketReadCallBack);
-        CFSocketDisableCallBacks(socketInfo->socket, kCFSocketWriteCallBack);
-
         macSockets.insert(nativeSocket, socketInfo);
     }
 
-    // Increment read/write counters and select enable callbacks if necessary.
     if (type == QSocketNotifier::Read) {
         Q_ASSERT(socketInfo->readNotifier == 0);
         socketInfo->readNotifier = notifier;
-        CFSocketEnableCallBacks(socketInfo->socket, kCFSocketReadCallBack);
+        socketInfo->readEnabled = false;
     } else if (type == QSocketNotifier::Write) {
         Q_ASSERT(socketInfo->writeNotifier == 0);
         socketInfo->writeNotifier = notifier;
-        CFSocketEnableCallBacks(socketInfo->socket, kCFSocketWriteCallBack);
+        socketInfo->writeEnabled = false;
+    }
+
+    if (!enableNotifiersObserver) {
+        // Create a run loop observer which enables the socket notifiers on each
+        // pass of the run loop, before any sources are processed.
+        CFRunLoopObserverContext context = {};
+        context.info = this;
+        enableNotifiersObserver = CFRunLoopObserverCreate(kCFAllocatorDefault, kCFRunLoopBeforeSources,
+                                                          true, 0, enableSocketNotifiers, &context);
+        Q_ASSERT(enableNotifiersObserver);
+        CFRunLoopAddObserver(CFRunLoopGetMain(), enableNotifiersObserver, kCFRunLoopCommonModes);
     }
 }
 
@@ -212,21 +214,18 @@ void QCFSocketNotifier::unregisterSocketNotifier(QSocketNotifier *notifier)
     if (type == QSocketNotifier::Read) {
         Q_ASSERT(notifier == socketInfo->readNotifier);
         socketInfo->readNotifier = 0;
+        socketInfo->readEnabled = false;
         CFSocketDisableCallBacks(socketInfo->socket, kCFSocketReadCallBack);
     } else if (type == QSocketNotifier::Write) {
         Q_ASSERT(notifier == socketInfo->writeNotifier);
         socketInfo->writeNotifier = 0;
+        socketInfo->writeEnabled = false;
         CFSocketDisableCallBacks(socketInfo->socket, kCFSocketWriteCallBack);
     }
 
     // Remove CFSocket from runloop if this was the last QSocketNotifier.
     if (socketInfo->readNotifier == 0 && socketInfo->writeNotifier == 0) {
-        if (CFSocketIsValid(socketInfo->socket))
-            qt_mac_remove_socket_from_runloop(socketInfo->socket, socketInfo->runloop);
-        CFRunLoopSourceInvalidate(socketInfo->runloop);
-        CFRelease(socketInfo->runloop);
-        CFSocketInvalidate(socketInfo->socket);
-        CFRelease(socketInfo->socket);
+        unregisterSocketInfo(socketInfo);
         delete socketInfo;
         macSockets.remove(nativeSocket);
     }
@@ -235,14 +234,70 @@ void QCFSocketNotifier::unregisterSocketNotifier(QSocketNotifier *notifier)
 void QCFSocketNotifier::removeSocketNotifiers()
 {
     // Remove CFSockets from the runloop.
-    for (MacSocketHash::ConstIterator it = macSockets.constBegin(); it != macSockets.constEnd(); ++it) {
-        MacSocketInfo *socketInfo = (*it);
-        if (CFSocketIsValid(socketInfo->socket)) {
+    foreach (MacSocketInfo *socketInfo, macSockets) {
+        unregisterSocketInfo(socketInfo);
+        delete socketInfo;
+    }
+
+    macSockets.clear();
+
+    destroyRunLoopObserver();
+}
+
+void QCFSocketNotifier::destroyRunLoopObserver()
+{
+    if (!enableNotifiersObserver)
+        return;
+
+    CFRunLoopObserverInvalidate(enableNotifiersObserver);
+    CFRelease(enableNotifiersObserver);
+    enableNotifiersObserver = 0;
+}
+
+void QCFSocketNotifier::unregisterSocketInfo(MacSocketInfo *socketInfo)
+{
+    if (socketInfo->runloop) {
+        if (CFSocketIsValid(socketInfo->socket))
             qt_mac_remove_socket_from_runloop(socketInfo->socket, socketInfo->runloop);
-            CFRunLoopSourceInvalidate(socketInfo->runloop);
-            CFRelease(socketInfo->runloop);
-            CFSocketInvalidate(socketInfo->socket);
-            CFRelease(socketInfo->socket);
+        CFRunLoopSourceInvalidate(socketInfo->runloop);
+        CFRelease(socketInfo->runloop);
+    }
+    CFSocketInvalidate(socketInfo->socket);
+    CFRelease(socketInfo->socket);
+}
+
+void QCFSocketNotifier::enableSocketNotifiers(CFRunLoopObserverRef ref, CFRunLoopActivity activity, void *info)
+{
+    Q_UNUSED(ref);
+    Q_UNUSED(activity);
+
+    QCFSocketNotifier *that = static_cast<QCFSocketNotifier *>(info);
+
+    foreach (MacSocketInfo *socketInfo, that->macSockets) {
+        if (!CFSocketIsValid(socketInfo->socket))
+            continue;
+
+        if (!socketInfo->runloop) {
+            // Add CFSocket to runloop.
+            if (!(socketInfo->runloop = qt_mac_add_socket_to_runloop(socketInfo->socket))) {
+                qWarning("QEventDispatcherMac::registerSocketNotifier: Failed to add CFSocket to runloop");
+                CFSocketInvalidate(socketInfo->socket);
+                continue;
+            }
+
+            if (!socketInfo->readNotifier)
+                CFSocketDisableCallBacks(socketInfo->socket, kCFSocketReadCallBack);
+            if (!socketInfo->writeNotifier)
+                CFSocketDisableCallBacks(socketInfo->socket, kCFSocketWriteCallBack);
+        }
+
+        if (socketInfo->readNotifier && !socketInfo->readEnabled) {
+            socketInfo->readEnabled = true;
+            CFSocketEnableCallBacks(socketInfo->socket, kCFSocketReadCallBack);
+        }
+        if (socketInfo->writeNotifier && !socketInfo->writeEnabled) {
+            socketInfo->writeEnabled = true;
+            CFSocketEnableCallBacks(socketInfo->socket, kCFSocketWriteCallBack);
         }
     }
 }

src/platformsupport/cfsocketnotifier/qcfsocketnotifier_p.h

@@ -53,11 +53,14 @@
 QT_BEGIN_NAMESPACE
 
 struct MacSocketInfo {
-    MacSocketInfo() : socket(0), runloop(0), readNotifier(0), writeNotifier(0) {}
+    MacSocketInfo() : socket(0), runloop(0), readNotifier(0), writeNotifier(0),
+        readEnabled(false), writeEnabled(false) {}
     CFSocketRef socket;
     CFRunLoopSourceRef runloop;
     QObject *readNotifier;
     QObject *writeNotifier;
+    bool readEnabled;
+    bool writeEnabled;
 };
 typedef QHash<int, MacSocketInfo *> MacSocketHash;
 
@@ -83,9 +86,18 @@ public:
     void unregisterSocketNotifier(QSocketNotifier *notifier);
     void removeSocketNotifiers();
 
+private:
+    void destroyRunLoopObserver();
+
+    static void unregisterSocketInfo(MacSocketInfo *socketInfo);
+    static void enableSocketNotifiers(CFRunLoopObserverRef ref, CFRunLoopActivity activity, void *info);
+
     MacSocketHash macSockets;
     QAbstractEventDispatcher *eventDispatcher;
     MaybeCancelWaitForMoreEventsFn maybeCancelWaitForMoreEvents;
+    CFRunLoopObserverRef enableNotifiersObserver;
+
+    friend void qt_mac_socket_callback(CFSocketRef, CFSocketCallBackType, CFDataRef, const void *, void *);
 };
 
 QT_END_NAMESPACE