Rework toggling the state of read notifier on buffered socket

Read notifications on a buffered socket are enabled all the time while
the socket is connected unless the buffer size limit is reached. To
enable the read notifier again, the user needs to read the data or
enlarge the buffer with a setReadBufferSize() call.

According to that scenario, it's very likely that the socket will
toggle the state of notifier twice (on->off->on) in one notification
cycle. Also, use of the readyRead() signal with a queued connection
leads to the same problem.

This patch prevents this unnecessary toggling by:
- unconditionally setting the notifier's state in setReadBufferSize();
- deferring the notification disabling in canReadNotification()
  to the next notification cycle.

Change-Id: I29df32f12c9c80a325373af86963e924817f2167
Reviewed-by: Oswald Buddenhagen <oswald.buddenhagen@theqtcompany.com>

1 files changed, 7 insertions, 15 deletions

diff --git a/src/network/socket/qabstractsocket.cpp b/src/network/socket/qabstractsocket.cpp
index a4ed5cc2df..0969d84180 100644
--- a/src/network/socket/qabstractsocket.cpp
+++ b/src/network/socket/qabstractsocket.cpp
@@ -691,6 +691,7 @@ bool QAbstractSocketPrivate::canReadNotification()
     if (isBuffered) {
         // Return if there is no space in the buffer
         if (readBufferMaxSize && buffer.size() >= readBufferMaxSize) {
+            socketEngine->setReadNotificationEnabled(false);
 #if defined (QABSTRACTSOCKET_DEBUG)
             qDebug("QAbstractSocketPrivate::canReadNotification() buffer is full");
 #endif
@@ -708,11 +709,6 @@ bool QAbstractSocketPrivate::canReadNotification()
             return false;
         }
         newBytes = buffer.size() - newBytes;
-
-        // If read buffer is full, disable the read socket notifier.
-        if (readBufferMaxSize && buffer.size() == readBufferMaxSize) {
-            socketEngine->setReadNotificationEnabled(false);
-        }
     }
 
     // Only emit readyRead() if there is data available.
@@ -728,10 +724,6 @@ bool QAbstractSocketPrivate::canReadNotification()
         return true;
     }
 
-    // turn the socket engine off if we've reached the buffer size limit
-    if (socketEngine && isBuffered)
-        socketEngine->setReadNotificationEnabled(readBufferMaxSize == 0 || readBufferMaxSize > q->bytesAvailable());
-
     return true;
 }
 
@@ -2784,12 +2776,12 @@ void QAbstractSocket::setReadBufferSize(qint64 size)
     if (d->readBufferMaxSize == size)
         return;
     d->readBufferMaxSize = size;
-    if (!d->emittedReadyRead && d->socketEngine) {
-        // ensure that the read notification is enabled if we've now got
-        // room in the read buffer
-        // but only if we're not inside canReadNotification -- that will take care on its own
-        if ((size == 0 || d->buffer.size() < size) && d->state == QAbstractSocket::ConnectedState) // Do not change the notifier unless we are connected.
-            d->socketEngine->setReadNotificationEnabled(true);
+
+    // Do not change the notifier unless we are connected.
+    if (d->socketEngine && d->state == QAbstractSocket::ConnectedState) {
+        // Ensure that the read notification is enabled if we've now got
+        // room in the read buffer.
+        d->socketEngine->setReadNotificationEnabled(size == 0 || d->buffer.size() < size);
     }
 }