From 9f5cdf1a217745f5138c7a8d479934b15be74197 Mon Sep 17 00:00:00 2001 From: Erik Duisters Date: Sun, 16 Dec 2018 17:16:22 +0100 Subject: [PATCH] Do not close m_socket in socketError() Summary: When android closes the payload socket (cancel share) calling m_socket.close() results in a recursive call to onError eventually leading to a segmentation violation Test Plan: Install D16491, share a large file from desktop to android and cancel the share on Android through the notification. kdeconnectd crashes almost 100% of the time. (It doesn't crash when it detects a disconnect in sendNextPacket) Reviewers: #kde_connect, nicolasfella Reviewed By: #kde_connect, nicolasfella Subscribers: kdeconnect Tags: #kde_connect Differential Revision: https://phabricator.kde.org/D17628 --- core/backends/lan/compositeuploadjob.cpp | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/core/backends/lan/compositeuploadjob.cpp b/core/backends/lan/compositeuploadjob.cpp index 874e0f2a4..581cc849b 100644 --- a/core/backends/lan/compositeuploadjob.cpp +++ b/core/backends/lan/compositeuploadjob.cpp @@ -117,7 +117,7 @@ void CompositeUploadJob::startNextSubJob() } else { setError(SendingNetworkPacketFailed); setErrorText(i18n("Failed to send packet to %1", Daemon::instance()->getDevice(m_deviceId)->name())); - //TODO: cleanup/resend remaining jobs + emitResult(); } } @@ -154,10 +154,10 @@ void CompositeUploadJob::socketError(QAbstractSocket::SocketError error) { Q_UNUSED(error); - m_socket->close(); + //Do not close the socket because when android closes the socket (share is cancelled) closing the socket leads to a cyclic socketError and eventually a segv setError(SocketError); emitResult(); - //TODO: cleanup jobs? + m_running = false; } @@ -168,7 +168,7 @@ void CompositeUploadJob::sslError(const QList& errors) m_socket->close(); setError(SslError); emitResult(); - //TODO: cleanup jobs? + m_running = false; } @@ -213,8 +213,6 @@ bool CompositeUploadJob::addSubjob(KJob* job) bool CompositeUploadJob::doKill() { - //TODO: Remove all subjobs? - //TODO: cleanup jobs? if (m_running) { m_running = false; @@ -241,9 +239,7 @@ void CompositeUploadJob::slotProcessedAmount(KJob *job, KJob::Unit unit, qulongl void CompositeUploadJob::slotResult(KJob *job) { //Copies job error and errorText and emits result if job is in error otherwise removes job from subjob list KCompositeJob::slotResult(job); - - //TODO: cleanup jobs? - + if (error() || !m_running) { return; }