Commit b8e0f7cf authored by Alex Trotsenko's avatar Alex Trotsenko Committed by Jani Heikkinen
Browse files

Fix the spurious socket notifications on OS X


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: default avatarMorten Johan Sørvig <morten.sorvig@theqtcompany.com>
Showing with 42 additions and 12 deletions
...@@ -55,11 +55,15 @@ void qt_mac_socket_callback(CFSocketRef s, CFSocketCallBackType callbackType, CF ...@@ -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 // notifier is now gone. The upshot is we have to check the notifier
// every time. // every time.
if (callbackType == kCFSocketReadCallBack) { if (callbackType == kCFSocketReadCallBack) {
if (socketInfo->readNotifier) if (socketInfo->readNotifier && socketInfo->readEnabled) {
socketInfo->readEnabled = false;
QGuiApplication::sendEvent(socketInfo->readNotifier, &notifierEvent); QGuiApplication::sendEvent(socketInfo->readNotifier, &notifierEvent);
}
} else if (callbackType == kCFSocketWriteCallBack) { } else if (callbackType == kCFSocketWriteCallBack) {
if (socketInfo->writeNotifier) if (socketInfo->writeNotifier && socketInfo->writeEnabled) {
socketInfo->writeEnabled = false;
QGuiApplication::sendEvent(socketInfo->writeNotifier, &notifierEvent); QGuiApplication::sendEvent(socketInfo->writeNotifier, &notifierEvent);
}
} }
if (cfSocketNotifier->maybeCancelWaitForMoreEvents) if (cfSocketNotifier->maybeCancelWaitForMoreEvents)
...@@ -150,8 +154,8 @@ void QCFSocketNotifier::registerSocketNotifier(QSocketNotifier *notifier) ...@@ -150,8 +154,8 @@ void QCFSocketNotifier::registerSocketNotifier(QSocketNotifier *notifier)
} }
CFOptionFlags flags = CFSocketGetSocketFlags(socketInfo->socket); CFOptionFlags flags = CFSocketGetSocketFlags(socketInfo->socket);
flags |= kCFSocketAutomaticallyReenableWriteCallBack; //QSocketNotifier stays enabled after a write // QSocketNotifier doesn't close the socket upon destruction/invalidation
flags &= ~kCFSocketCloseOnInvalidate; //QSocketNotifier doesn't close the socket upon destruction/invalidation flags &= ~(kCFSocketCloseOnInvalidate | kCFSocketAutomaticallyReenableReadCallBack);
CFSocketSetSocketFlags(socketInfo->socket, flags); CFSocketSetSocketFlags(socketInfo->socket, flags);
// Add CFSocket to runloop. // Add CFSocket to runloop.
...@@ -171,15 +175,14 @@ void QCFSocketNotifier::registerSocketNotifier(QSocketNotifier *notifier) ...@@ -171,15 +175,14 @@ void QCFSocketNotifier::registerSocketNotifier(QSocketNotifier *notifier)
macSockets.insert(nativeSocket, socketInfo); macSockets.insert(nativeSocket, socketInfo);
} }
// Increment read/write counters and select enable callbacks if necessary.
if (type == QSocketNotifier::Read) { if (type == QSocketNotifier::Read) {
Q_ASSERT(socketInfo->readNotifier == 0); Q_ASSERT(socketInfo->readNotifier == 0);
socketInfo->readNotifier = notifier; socketInfo->readNotifier = notifier;
CFSocketEnableCallBacks(socketInfo->socket, kCFSocketReadCallBack); socketInfo->readEnabled = false;
} else if (type == QSocketNotifier::Write) { } else if (type == QSocketNotifier::Write) {
Q_ASSERT(socketInfo->writeNotifier == 0); Q_ASSERT(socketInfo->writeNotifier == 0);
socketInfo->writeNotifier = notifier; socketInfo->writeNotifier = notifier;
CFSocketEnableCallBacks(socketInfo->socket, kCFSocketWriteCallBack); socketInfo->writeEnabled = false;
} }
} }
...@@ -212,10 +215,12 @@ void QCFSocketNotifier::unregisterSocketNotifier(QSocketNotifier *notifier) ...@@ -212,10 +215,12 @@ void QCFSocketNotifier::unregisterSocketNotifier(QSocketNotifier *notifier)
if (type == QSocketNotifier::Read) { if (type == QSocketNotifier::Read) {
Q_ASSERT(notifier == socketInfo->readNotifier); Q_ASSERT(notifier == socketInfo->readNotifier);
socketInfo->readNotifier = 0; socketInfo->readNotifier = 0;
socketInfo->readEnabled = false;
CFSocketDisableCallBacks(socketInfo->socket, kCFSocketReadCallBack); CFSocketDisableCallBacks(socketInfo->socket, kCFSocketReadCallBack);
} else if (type == QSocketNotifier::Write) { } else if (type == QSocketNotifier::Write) {
Q_ASSERT(notifier == socketInfo->writeNotifier); Q_ASSERT(notifier == socketInfo->writeNotifier);
socketInfo->writeNotifier = 0; socketInfo->writeNotifier = 0;
socketInfo->writeEnabled = false;
CFSocketDisableCallBacks(socketInfo->socket, kCFSocketWriteCallBack); CFSocketDisableCallBacks(socketInfo->socket, kCFSocketWriteCallBack);
} }
...@@ -232,6 +237,24 @@ void QCFSocketNotifier::unregisterSocketNotifier(QSocketNotifier *notifier) ...@@ -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() void QCFSocketNotifier::removeSocketNotifiers()
{ {
// Remove CFSockets from the runloop. // Remove CFSockets from the runloop.
......
...@@ -53,11 +53,14 @@ ...@@ -53,11 +53,14 @@
QT_BEGIN_NAMESPACE QT_BEGIN_NAMESPACE
struct MacSocketInfo { 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; CFSocketRef socket;
CFRunLoopSourceRef runloop; CFRunLoopSourceRef runloop;
QObject *readNotifier; QObject *readNotifier;
QObject *writeNotifier; QObject *writeNotifier;
bool readEnabled;
bool writeEnabled;
}; };
typedef QHash<int, MacSocketInfo *> MacSocketHash; typedef QHash<int, MacSocketInfo *> MacSocketHash;
...@@ -81,6 +84,7 @@ public: ...@@ -81,6 +84,7 @@ public:
void setMaybeCancelWaitForMoreEventsCallback(MaybeCancelWaitForMoreEventsFn callBack); void setMaybeCancelWaitForMoreEventsCallback(MaybeCancelWaitForMoreEventsFn callBack);
void registerSocketNotifier(QSocketNotifier *notifier); void registerSocketNotifier(QSocketNotifier *notifier);
void unregisterSocketNotifier(QSocketNotifier *notifier); void unregisterSocketNotifier(QSocketNotifier *notifier);
void enableSocketNotifiers();
void removeSocketNotifiers(); void removeSocketNotifiers();
MacSocketHash macSockets; MacSocketHash macSockets;
......
...@@ -845,10 +845,13 @@ QCocoaEventDispatcher::QCocoaEventDispatcher(QObject *parent) ...@@ -845,10 +845,13 @@ QCocoaEventDispatcher::QCocoaEventDispatcher(QObject *parent)
void QCocoaEventDispatcherPrivate::waitingObserverCallback(CFRunLoopObserverRef, void QCocoaEventDispatcherPrivate::waitingObserverCallback(CFRunLoopObserverRef,
CFRunLoopActivity activity, void *info) CFRunLoopActivity activity, void *info)
{ {
if (activity == kCFRunLoopBeforeWaiting) QCocoaEventDispatcher *dispatcher = static_cast<QCocoaEventDispatcher *>(info);
emit static_cast<QCocoaEventDispatcher*>(info)->aboutToBlock(); if (activity == kCFRunLoopBeforeWaiting) {
else dispatcher->d_func()->cfSocketNotifier.enableSocketNotifiers();
emit static_cast<QCocoaEventDispatcher*>(info)->awake(); emit dispatcher->aboutToBlock();
} else {
emit dispatcher->awake();
}
} }
void QCocoaEventDispatcherPrivate::processPostedEvents() void QCocoaEventDispatcherPrivate::processPostedEvents()
......
Supports Markdown
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment