Commit c06e4097 authored by Kevin Funk's avatar Kevin Funk Committed by Kevin Funk

Windows: Handle QLocalSocket behavior gracefully

Summary:
On Windows the underlying implementation of QLocalSocket behaves
differently, it seems to trigger the QLocalSocket::readyRead() signal
earlier than anticipated (as on Linux).
Prior to this patch this was not handled gracefully.

Kudos to David Faure for actually figuring this out by just looking at
the code and my debug output on Windows. :)

BEFORE:
Let's have a look at what happens on Windows inside Akonadi's Conneciton
class:

- Akonadi data sends Hello cmd => 91 bytes
- Akonadi resource receives data
- QLocalSocket::readyReady() signal fires
- Triggers Connection::handleIncomingData via signal-slot connection
- On Windows, QLocalSocket::bytesAvailable() is just 8 bytes
- Only the first 8 bytes are read, into the `qint64 tag` variable
- Protocol::deserialize(...) is called
  - Internally calls waitForData(...) (=> waitForReadyRead()) a few times
- QLocalSocket::readyRead() signal fires again
- Connection::handleIncomingData re-entered => BUG

Problematic end result:
```
[10972] org.kde.pim.akonadicore: tag: -1099511627647
[10972] org.kde.pim.akonadicore: Invalid command, the world is going to end!
[10972] org.kde.pim.akonadicore: State changed: QLocalSocket::ClosingState
[10972] org.kde.pim.akonadicore: State changed: QLocalSocket::UnconnectedState
```

=> Resource attempts reconnection to server, just to fail again
afterwards.

AFTER:
The fix is to temporarily disconnect from the readyRead() signal while
attempting to wait for data to deserialize commands. This in order to never
re-enter Connection::handleIncomingData() while doing so.

Reviewers: dfaure, mlaurent, dvratil

Reviewed By: dvratil

Subscribers: dvratil, kde-pim

Tags: #kde_pim

Differential Revision: https://phabricator.kde.org/D29266
parent 1a13474c
......@@ -180,6 +180,7 @@ void Connection::doReconnect()
Q_EMIT socketDisconnected();
});
connect(mSocket.data(), &QLocalSocket::disconnected, this, &Connection::socketDisconnected);
// note: we temporarily disconnect from readyRead-signal inside handleIncomingData()
connect(mSocket.data(), &QLocalSocket::readyRead, this, &Connection::handleIncomingData);
// actually do connect
......@@ -253,6 +254,10 @@ void Connection::handleIncomingData()
qint64 tag;
stream >> tag;
// temporarily disconnect from readyRead-signal to avoid re-entering this function when we
// call waitForData() deep inside Protocol::deserialize
disconnect(mSocket.data(), &QLocalSocket::readyRead, this, &Connection::handleIncomingData);
Protocol::CommandPtr cmd;
try {
cmd = Protocol::deserialize(mSocket.data());
......@@ -260,6 +265,10 @@ void Connection::handleIncomingData()
qCWarning(AKONADICORE_LOG) << "Protocol exception:" << e.what();
// cmd's type will be Invalid by default, so fall-through
}
// reconnect to the signal again
connect(mSocket.data(), &QLocalSocket::readyRead, this, &Connection::handleIncomingData);
if (!cmd || (cmd->type() == Protocol::Command::Invalid)) {
qCWarning(AKONADICORE_LOG) << "Invalid command, the world is going to end!";
mSocket->close();
......
Markdown is supported
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