From b8e0f7cfc638a71770f44ada828ff2cf6d2ee201 Mon Sep 17 00:00:00 2001
From: Alex Trotsenko <alex1973tr@gmail.com>
Date: Thu, 1 Oct 2015 17:41:07 +0300
Subject: [PATCH] Fix the spurious socket notifications on OS X
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Core Foundation Framework forwards notifications about socket activity
through a callback function which called from the run loop. Previous
implementation sets kCFSocketReadCallBack, kCFSocketWriteCallBack to be
automatically re-enabled after they are triggered. With these semantics,
an application need not read all available data in response to a read
notification: a single recv in response to each read notification is
appropriate. If an application issues multiple recv calls in response to
a single notification, it can receive spurious notifications.

To solve this issue, this patch disables automatically reenabling callback
feature. Now, callback gets called exactly once, and is not called again
until manually re-enabled by calling CFSocketEnableCallBacks() just before
entering to wait for the new events.

Task-number: QTBUG-48556
Change-Id: Ia3393c2026230c7b3397cc614758dec1d432535f
Reviewed-by: Morten Johan Sørvig <morten.sorvig@theqtcompany.com>
---
 .../cfsocketnotifier/qcfsocketnotifier.cpp    | 37 +++++++++++++++----
 .../cfsocketnotifier/qcfsocketnotifier_p.h    |  6 ++-
 .../platforms/cocoa/qcocoaeventdispatcher.mm  | 11 ++++--
 3 files changed, 42 insertions(+), 12 deletions(-)

diff --git a/src/platformsupport/cfsocketnotifier/qcfsocketnotifier.cpp b/src/platformsupport/cfsocketnotifier/qcfsocketnotifier.cpp
index 2b5723d8277..91f7aeb7d03 100644
--- a/src/platformsupport/cfsocketnotifier/qcfsocketnotifier.cpp
+++ b/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)
@@ -150,8 +154,8 @@ 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 | kCFSocketAutomaticallyReenableReadCallBack);
         CFSocketSetSocketFlags(socketInfo->socket, flags);
 
         // Add CFSocket to runloop.
@@ -171,15 +175,14 @@ void QCFSocketNotifier::registerSocketNotifier(QSocketNotifier *notifier)
         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;
     }
 }
 
@@ -212,10 +215,12 @@ 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);
     }
 
@@ -232,6 +237,24 @@ void QCFSocketNotifier::unregisterSocketNotifier(QSocketNotifier *notifier)
     }
 }
 
+void QCFSocketNotifier::enableSocketNotifiers()
+{
+    // Enable CFSockets in runloop.
+    for (MacSocketHash::ConstIterator it = macSockets.constBegin(); it != macSockets.constEnd(); ++it) {
+        MacSocketInfo *socketInfo = (*it);
+        if (CFSocketIsValid(socketInfo->socket)) {
+            if (socketInfo->readNotifier && !socketInfo->readEnabled) {
+                socketInfo->readEnabled = true;
+                CFSocketEnableCallBacks(socketInfo->socket, kCFSocketReadCallBack);
+            }
+            if (socketInfo->writeNotifier && !socketInfo->writeEnabled) {
+                socketInfo->writeEnabled = true;
+                CFSocketEnableCallBacks(socketInfo->socket, kCFSocketWriteCallBack);
+            }
+        }
+    }
+}
+
 void QCFSocketNotifier::removeSocketNotifiers()
 {
     // Remove CFSockets from the runloop.
diff --git a/src/platformsupport/cfsocketnotifier/qcfsocketnotifier_p.h b/src/platformsupport/cfsocketnotifier/qcfsocketnotifier_p.h
index af8122f753a..1d6dcf28850 100644
--- a/src/platformsupport/cfsocketnotifier/qcfsocketnotifier_p.h
+++ b/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;
 
@@ -81,6 +84,7 @@ public:
     void setMaybeCancelWaitForMoreEventsCallback(MaybeCancelWaitForMoreEventsFn callBack);
     void registerSocketNotifier(QSocketNotifier *notifier);
     void unregisterSocketNotifier(QSocketNotifier *notifier);
+    void enableSocketNotifiers();
     void removeSocketNotifiers();
 
     MacSocketHash macSockets;
diff --git a/src/plugins/platforms/cocoa/qcocoaeventdispatcher.mm b/src/plugins/platforms/cocoa/qcocoaeventdispatcher.mm
index 52b2e23345a..b443233d154 100644
--- a/src/plugins/platforms/cocoa/qcocoaeventdispatcher.mm
+++ b/src/plugins/platforms/cocoa/qcocoaeventdispatcher.mm
@@ -845,10 +845,13 @@ QCocoaEventDispatcher::QCocoaEventDispatcher(QObject *parent)
 void QCocoaEventDispatcherPrivate::waitingObserverCallback(CFRunLoopObserverRef,
                                                           CFRunLoopActivity activity, void *info)
 {
-    if (activity == kCFRunLoopBeforeWaiting)
-        emit static_cast<QCocoaEventDispatcher*>(info)->aboutToBlock();
-    else
-        emit static_cast<QCocoaEventDispatcher*>(info)->awake();
+    QCocoaEventDispatcher *dispatcher = static_cast<QCocoaEventDispatcher *>(info);
+    if (activity == kCFRunLoopBeforeWaiting) {
+        dispatcher->d_func()->cfSocketNotifier.enableSocketNotifiers();
+        emit dispatcher->aboutToBlock();
+    } else {
+        emit dispatcher->awake();
+    }
 }
 
 void QCocoaEventDispatcherPrivate::processPostedEvents()
-- 
GitLab