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.
This commit is contained in:
jwestfall 2018-12-28 14:03:42 -08:00
parent 3d835ea4aa
commit 94f8336af5
1 changed files with 15 additions and 8 deletions

View File

@ -544,10 +544,11 @@ TCPSocket::serviceConnected(ISocketMultiplexerJob* job,
return newJob(); return newJob();
} }
EJobResult result = kRetry; EJobResult writeResult = kRetry;
EJobResult readResult = kRetry;
if (write) { if (write) {
try { try {
result = doWrite(); writeResult = doWrite();
} }
catch (XArchNetworkShutdown&) { catch (XArchNetworkShutdown&) {
// remote read end of stream hungup. our output side // remote read end of stream hungup. our output side
@ -558,13 +559,13 @@ TCPSocket::serviceConnected(ISocketMultiplexerJob* job,
sendEvent(m_events->forISocket().disconnected()); sendEvent(m_events->forISocket().disconnected());
m_connected = false; m_connected = false;
} }
result = kNew; writeResult = kNew;
} }
catch (XArchNetworkDisconnected&) { catch (XArchNetworkDisconnected&) {
// stream hungup // stream hungup
onDisconnected(); onDisconnected();
sendEvent(m_events->forISocket().disconnected()); sendEvent(m_events->forISocket().disconnected());
result = kNew; writeResult = kNew;
} }
catch (XArchNetwork& e) { catch (XArchNetwork& e) {
// other write error // other write error
@ -572,19 +573,19 @@ TCPSocket::serviceConnected(ISocketMultiplexerJob* job,
onDisconnected(); onDisconnected();
sendEvent(m_events->forIStream().outputError()); sendEvent(m_events->forIStream().outputError());
sendEvent(m_events->forISocket().disconnected()); sendEvent(m_events->forISocket().disconnected());
result = kNew; writeResult = kNew;
} }
} }
if (read && m_readable) { if (read && m_readable) {
try { try {
result = doRead(); readResult = doRead();
} }
catch (XArchNetworkDisconnected&) { catch (XArchNetworkDisconnected&) {
// stream hungup // stream hungup
sendEvent(m_events->forISocket().disconnected()); sendEvent(m_events->forISocket().disconnected());
onDisconnected(); onDisconnected();
result = kNew; readResult = kNew;
} }
catch (XArchNetwork& e) { catch (XArchNetwork& e) {
// ignore other read error // 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;
}
} }