From af778b71b166c5b1850d083a6bfa375f39d91467 Mon Sep 17 00:00:00 2001 From: Albert Astals Cid Date: Tue, 1 Aug 2017 19:40:43 +0200 Subject: [PATCH] No need to use a QBuffer in DownloadJob Summary: QSslSocket is already a QIODevice so just use that. Should fix the issue of transfering *big* files. Unfortunately this seems to trigger a bug in KIO and CPU usage goes through the roof, so haven't really been able to test it does actually fix things. Please don't merge/approve yet Reviewers: apol, albertvaka, kdeconnect Subscribers: #kde_connect Differential Revision: https://phabricator.kde.org/D6039 --- core/backends/lan/downloadjob.cpp | 41 +++++++++++++++++++------------ core/backends/lan/downloadjob.h | 3 +-- core/filetransferjob.cpp | 4 +++ 3 files changed, 30 insertions(+), 18 deletions(-) diff --git a/core/backends/lan/downloadjob.cpp b/core/backends/lan/downloadjob.cpp index 8db5901fb..7e573d219 100644 --- a/core/backends/lan/downloadjob.cpp +++ b/core/backends/lan/downloadjob.cpp @@ -31,17 +31,30 @@ #include "lanlinkprovider.h" #include "core/core_debug.h" +class QQSslSocket : public QSslSocket +{ +private: + // Workaround Qt bug https://codereview.qt-project.org/#/c/195723/ + qint64 readData(char *data, qint64 maxSize) override { + qint64 res = QSslSocket::readData(data, maxSize); + if (res == 0 && encryptedBytesAvailable() == 0 && state() != ConnectedState) { + return maxSize ? qint64(-1) : qint64(0); + } else { + return res; + } + } +}; + DownloadJob::DownloadJob(const QHostAddress &address, const QVariantMap &transferInfo) : KJob() , mAddress(address) , mPort(transferInfo[QStringLiteral("port")].toInt()) - , mSocket(new QSslSocket) - , mBuffer(new QBuffer) + , mSocket(new QQSslSocket) { LanLinkProvider::configureSslSocket(mSocket.data(), transferInfo.value(QStringLiteral("deviceId")).toString(), true); connect(mSocket.data(), SIGNAL(error(QAbstractSocket::SocketError)), this, SLOT(socketFailed(QAbstractSocket::SocketError))); -// connect(mSocket.data(), &QAbstractSocket::stateChanged, [](QAbstractSocket::SocketState state){ qDebug() << "statechange" << state; }); + connect(mSocket.data(), &QAbstractSocket::connected, this, &DownloadJob::socketConnected); } DownloadJob::~DownloadJob() @@ -54,26 +67,22 @@ void DownloadJob::start() //TODO: Timeout? // Cannot use read only, might be due to ssl handshake, getting QIODevice::ReadOnly error and no connection mSocket->connectToHostEncrypted(mAddress.toString(), mPort, QIODevice::ReadWrite); - - bool b = mBuffer->open(QBuffer::ReadWrite); - Q_ASSERT(b); } void DownloadJob::socketFailed(QAbstractSocket::SocketError error) { - if (error != QAbstractSocket::RemoteHostClosedError) { //remote host closes when finishes - qWarning(KDECONNECT_CORE) << "error..." << mSocket->errorString(); - setError(error + 1); - setErrorText(mSocket->errorString()); - } else { - auto ba = mSocket->readAll(); - mBuffer->write(ba); - mBuffer->seek(0); - } + qWarning() << "error..." << (error == QAbstractSocket::RemoteHostClosedError) << mSocket->errorString(); + setError(error + 1); + setErrorText(mSocket->errorString()); emitResult(); } QSharedPointer DownloadJob::getPayload() { - return mBuffer.staticCast(); + return mSocket.staticCast(); +} + +void DownloadJob::socketConnected() +{ + emitResult(); } diff --git a/core/backends/lan/downloadjob.h b/core/backends/lan/downloadjob.h index daa898eb2..3876e8c5e 100644 --- a/core/backends/lan/downloadjob.h +++ b/core/backends/lan/downloadjob.h @@ -47,11 +47,10 @@ private: QHostAddress mAddress; qint16 mPort; QSharedPointer mSocket; - QSharedPointer mBuffer; private Q_SLOTS: void socketFailed(QAbstractSocket::SocketError error); - + void socketConnected(); }; #endif // UPLOADJOB_H diff --git a/core/filetransferjob.cpp b/core/filetransferjob.cpp index 8bb9e7ed3..3089b4c27 100644 --- a/core/filetransferjob.cpp +++ b/core/filetransferjob.cpp @@ -79,6 +79,10 @@ void FileTransferJob::doStart() void FileTransferJob::startTransfer() { + // Don't put each ready read + if (mTimer.isValid()) + return; + setProcessedAmount(Bytes, 0); mTimer.start(); description(this, i18n("Receiving file over KDE Connect"),