From a0b81614bc849a8ce46bf5f69d108e264962d332 Mon Sep 17 00:00:00 2001
From: Peter Seiderer <ps.report@gmx.net>
Date: Mon, 15 Jun 2015 22:51:20 +0200
Subject: [PATCH v1] WIP: Fix QUdpSocket readyRead()/hasPendingDatagrams() race
 condition.

Querying hasPendingDatagrams() for enabling/disabling setReadNotificationEnabled()
is racy (a new datagram could arrive after readDatagam() is called and before
hasPendingDatagrams() is checked), better store if last datagram is already
signaled and reset in case readDatagram() is called.

Implemented only for qnativesocketengine (dummy implementations
for qhttpsocketengine (o.k.?) and qsocks5socketengine (did not
yet analyze)).

Task-number: QTBUG-46552
Change-Id: Ib433e2f777bf4c143d19fffb3215d8d4ba12178c
---
 src/network/socket/qabstractsocket.cpp       |  3 ++-
 src/network/socket/qabstractsocketengine_p.h |  2 ++
 src/network/socket/qhttpsocketengine.cpp     | 10 ++++++++++
 src/network/socket/qhttpsocketengine_p.h     |  2 ++
 src/network/socket/qnativesocketengine.cpp   | 16 +++++++++++++++-
 src/network/socket/qnativesocketengine_p.h   |  4 ++++
 src/network/socket/qsocks5socketengine.cpp   | 10 ++++++++++
 src/network/socket/qsocks5socketengine_p.h   |  2 ++
 8 files changed, 47 insertions(+), 2 deletions(-)

diff --git a/src/network/socket/qabstractsocket.cpp b/src/network/socket/qabstractsocket.cpp
index 5a1ad40b90..24221b273a 100644
--- a/src/network/socket/qabstractsocket.cpp
+++ b/src/network/socket/qabstractsocket.cpp
@@ -728,6 +728,7 @@ bool QAbstractSocketPrivate::canReadNotification()
     if (!emittedReadyRead && hasData) {
         QScopedValueRollback<bool> r(emittedReadyRead);
         emittedReadyRead = true;
+        socketEngine->setPendingDatagramSignaled();
         emit q->readyRead();
     }
 
@@ -747,7 +748,7 @@ bool QAbstractSocketPrivate::canReadNotification()
         if (isBuffered)
             socketEngine->setReadNotificationEnabled(readBufferMaxSize == 0 || readBufferMaxSize > q->bytesAvailable());
         else if (socketType != QAbstractSocket::TcpSocket)
-            socketEngine->setReadNotificationEnabled(!socketEngine->hasPendingDatagrams());
+            socketEngine->setReadNotificationEnabled(!socketEngine->pendingDatagramSignaled());
     }
 
     // reset the read socket notifier state if we reentered inside the
diff --git a/src/network/socket/qabstractsocketengine_p.h b/src/network/socket/qabstractsocketengine_p.h
index cde7c1159d..aa47423979 100644
--- a/src/network/socket/qabstractsocketengine_p.h
+++ b/src/network/socket/qabstractsocketengine_p.h
@@ -132,6 +132,8 @@ public:
                                  quint16 port) = 0;
     virtual bool hasPendingDatagrams() const = 0;
     virtual qint64 pendingDatagramSize() const = 0;
+    virtual bool pendingDatagramSignaled() const = 0;
+    virtual void setPendingDatagramSignaled() = 0;
 #endif // QT_NO_UDPSOCKET
 
     virtual qint64 bytesToWrite() const = 0;
diff --git a/src/network/socket/qhttpsocketengine.cpp b/src/network/socket/qhttpsocketengine.cpp
index b929ee088e..2019537f9c 100644
--- a/src/network/socket/qhttpsocketengine.cpp
+++ b/src/network/socket/qhttpsocketengine.cpp
@@ -291,6 +291,16 @@ qint64 QHttpSocketEngine::pendingDatagramSize() const
 {
     return 0;
 }
+
+bool QHttpSocketEngine::pendingDatagramSignaled() const
+{
+    return false;
+}
+
+void QHttpSocketEngine::setPendingDatagramSignaled()
+{
+
+}
 #endif // QT_NO_UDPSOCKET
 
 qint64 QHttpSocketEngine::bytesToWrite() const
diff --git a/src/network/socket/qhttpsocketengine_p.h b/src/network/socket/qhttpsocketengine_p.h
index 0448293981..a493c1bbf9 100644
--- a/src/network/socket/qhttpsocketengine_p.h
+++ b/src/network/socket/qhttpsocketengine_p.h
@@ -111,6 +111,8 @@ public:
         quint16 port);
     bool hasPendingDatagrams() const;
     qint64 pendingDatagramSize() const;
+    bool pendingDatagramSignaled() const;
+    void setPendingDatagramSignaled();
 #endif // QT_NO_UDPSOCKET
 
     qint64 bytesToWrite() const;
diff --git a/src/network/socket/qnativesocketengine.cpp b/src/network/socket/qnativesocketengine.cpp
index 63f50632d2..c87a18ff27 100644
--- a/src/network/socket/qnativesocketengine.cpp
+++ b/src/network/socket/qnativesocketengine.cpp
@@ -155,7 +155,8 @@ QNativeSocketEnginePrivate::QNativeSocketEnginePrivate() :
     socketDescriptor(-1),
     readNotifier(0),
     writeNotifier(0),
-    exceptNotifier(0)
+    exceptNotifier(0),
+    isPendingDatagramSignaled(false)
 {
 }
 
@@ -721,6 +722,18 @@ qint64 QNativeSocketEngine::pendingDatagramSize() const
     return d->nativePendingDatagramSize();
 }
 
+bool QNativeSocketEngine::pendingDatagramSignaled() const
+{
+    Q_D(const QNativeSocketEngine);
+    return d->isPendingDatagramSignaled;
+}
+
+void QNativeSocketEngine::setPendingDatagramSignaled()
+{
+    Q_D(QNativeSocketEngine);
+    d->isPendingDatagramSignaled = true;
+}
+
 /*!
     Reads up to \a maxSize bytes of a datagram from the socket,
     stores it in \a data and returns the number of bytes read. The
@@ -743,6 +756,7 @@ qint64 QNativeSocketEngine::readDatagram(char *data, qint64 maxSize, QHostAddres
     Q_CHECK_VALID_SOCKETLAYER(QNativeSocketEngine::readDatagram(), -1);
     Q_CHECK_TYPE(QNativeSocketEngine::readDatagram(), QAbstractSocket::UdpSocket, -1);
 
+    d->isPendingDatagramSignaled = false;
     return d->nativeReceiveDatagram(data, maxSize, address, port);
 }
 
diff --git a/src/network/socket/qnativesocketengine_p.h b/src/network/socket/qnativesocketengine_p.h
index 67412ca326..062d1ba90f 100644
--- a/src/network/socket/qnativesocketengine_p.h
+++ b/src/network/socket/qnativesocketengine_p.h
@@ -139,6 +139,8 @@ public:
                              quint16 port);
     bool hasPendingDatagrams() const;
     qint64 pendingDatagramSize() const;
+    bool pendingDatagramSignaled() const;
+    void setPendingDatagramSignaled();
 
     qint64 bytesToWrite() const;
 
@@ -196,6 +198,8 @@ public:
 
     QSocketNotifier *readNotifier, *writeNotifier, *exceptNotifier;
 
+    bool isPendingDatagramSignaled;
+
 #ifdef Q_OS_WIN
     QWindowsSockInit winSock;
 #endif
diff --git a/src/network/socket/qsocks5socketengine.cpp b/src/network/socket/qsocks5socketengine.cpp
index 1634352333..d2c57a914d 100644
--- a/src/network/socket/qsocks5socketengine.cpp
+++ b/src/network/socket/qsocks5socketengine.cpp
@@ -1687,6 +1687,16 @@ qint64 QSocks5SocketEngine::pendingDatagramSize() const
         return d->udpData->pendingDatagrams.head().data.size();
     return 0;
 }
+
+bool QSocks5SocketEngine::pendingDatagramSignaled() const
+{
+    return false;
+}
+
+void QSocks5SocketEngine::setPendingDatagramSignaled()
+{
+
+}
 #endif // QT_NO_UDPSOCKET
 
 qint64 QSocks5SocketEngine::bytesToWrite() const
diff --git a/src/network/socket/qsocks5socketengine_p.h b/src/network/socket/qsocks5socketengine_p.h
index fd6e187b98..379aa87feb 100644
--- a/src/network/socket/qsocks5socketengine_p.h
+++ b/src/network/socket/qsocks5socketengine_p.h
@@ -99,6 +99,8 @@ public:
         quint16 port);
     bool hasPendingDatagrams() const;
     qint64 pendingDatagramSize() const;
+    bool pendingDatagramSignaled() const;
+    void setPendingDatagramSignaled();
 #endif // QT_NO_UDPSOCKET
 
     qint64 bytesToWrite() const;
-- 
2.11.0

