From 94f8336af546c724f4be21f41df766284899247a Mon Sep 17 00:00:00 2001
From: jwestfall <jwestfall@surrealistic.net>
Date: Fri, 28 Dec 2018 14:03:42 -0800
Subject: [PATCH] Properly deal with a socket that is readable and writable at
 the same time

Its possible poll() will return that a socket is both readable and
writable.  When this happens TCPSocket::serviceConnected() is
overwriting the result from doWrite() with the result from doRead()

This can cause a situation where doWrite() tried to notify that we
should stop polling if the socket is writable, but the doRead() result
causes that to be ignored.  This results in a tight loop and 100% cpu
usage in SocketMultiplexer::serviceThread() as the poll() call
instantly returns that the socket is writable, but no one cares that
it is.  The issue eventually corrects itself with enough mouse
movement/clicks.
---
 src/lib/net/TCPSocket.cpp | 23 +++++++++++++++--------
 1 file changed, 15 insertions(+), 8 deletions(-)

diff --git a/src/lib/net/TCPSocket.cpp b/src/lib/net/TCPSocket.cpp
index dce81eef..d454aa1f 100644
--- a/src/lib/net/TCPSocket.cpp
+++ b/src/lib/net/TCPSocket.cpp
@@ -544,10 +544,11 @@ TCPSocket::serviceConnected(ISocketMultiplexerJob* job,
         return newJob();
     }
 
-    EJobResult result = kRetry;
+    EJobResult writeResult = kRetry;
+    EJobResult readResult = kRetry;
     if (write) {
         try {
-            result = doWrite();
+            writeResult = doWrite();
         }
         catch (XArchNetworkShutdown&) {
             // remote read end of stream hungup.  our output side
@@ -558,13 +559,13 @@ TCPSocket::serviceConnected(ISocketMultiplexerJob* job,
                 sendEvent(m_events->forISocket().disconnected());
                 m_connected = false;
             }
-            result = kNew;
+            writeResult = kNew;
         }
         catch (XArchNetworkDisconnected&) {
             // stream hungup
             onDisconnected();
             sendEvent(m_events->forISocket().disconnected());
-            result = kNew;
+            writeResult = kNew;
         }
         catch (XArchNetwork& e) {
             // other write error
@@ -572,19 +573,19 @@ TCPSocket::serviceConnected(ISocketMultiplexerJob* job,
             onDisconnected();
             sendEvent(m_events->forIStream().outputError());
             sendEvent(m_events->forISocket().disconnected());
-            result = kNew;
+            writeResult = kNew;
         }
     }
 
     if (read && m_readable) {
         try {
-            result = doRead();
+            readResult = doRead();
         }
         catch (XArchNetworkDisconnected&) {
             // stream hungup
             sendEvent(m_events->forISocket().disconnected());
             onDisconnected();
-            result = kNew;
+            readResult = kNew;
         }
         catch (XArchNetwork& e) {
             // ignore other read error
@@ -592,5 +593,11 @@ TCPSocket::serviceConnected(ISocketMultiplexerJob* job,
         }
     }
 
-    return result == kBreak ? NULL : result == kNew ? newJob() : job;
+    if (writeResult == kBreak || readResult == kBreak) {
+        return NULL;
+    } else if (writeResult == kNew || readResult == kNew) {
+        return newJob();
+    } else {
+        return job;
+    }
 }