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
This commit is contained in:
Erik Duisters 2018-12-16 17:16:22 +01:00
parent 71c5a1b248
commit 9f5cdf1a21

View file

@ -117,7 +117,7 @@ void CompositeUploadJob::startNextSubJob()
} else { } else {
setError(SendingNetworkPacketFailed); setError(SendingNetworkPacketFailed);
setErrorText(i18n("Failed to send packet to %1", Daemon::instance()->getDevice(m_deviceId)->name())); setErrorText(i18n("Failed to send packet to %1", Daemon::instance()->getDevice(m_deviceId)->name()));
//TODO: cleanup/resend remaining jobs
emitResult(); emitResult();
} }
} }
@ -154,10 +154,10 @@ void CompositeUploadJob::socketError(QAbstractSocket::SocketError error)
{ {
Q_UNUSED(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); setError(SocketError);
emitResult(); emitResult();
//TODO: cleanup jobs?
m_running = false; m_running = false;
} }
@ -168,7 +168,7 @@ void CompositeUploadJob::sslError(const QList<QSslError>& errors)
m_socket->close(); m_socket->close();
setError(SslError); setError(SslError);
emitResult(); emitResult();
//TODO: cleanup jobs?
m_running = false; m_running = false;
} }
@ -213,8 +213,6 @@ bool CompositeUploadJob::addSubjob(KJob* job)
bool CompositeUploadJob::doKill() bool CompositeUploadJob::doKill()
{ {
//TODO: Remove all subjobs?
//TODO: cleanup jobs?
if (m_running) { if (m_running) {
m_running = false; m_running = false;
@ -241,9 +239,7 @@ void CompositeUploadJob::slotProcessedAmount(KJob *job, KJob::Unit unit, qulongl
void CompositeUploadJob::slotResult(KJob *job) { void CompositeUploadJob::slotResult(KJob *job) {
//Copies job error and errorText and emits result if job is in error otherwise removes job from subjob list //Copies job error and errorText and emits result if job is in error otherwise removes job from subjob list
KCompositeJob::slotResult(job); KCompositeJob::slotResult(job);
//TODO: cleanup jobs?
if (error() || !m_running) { if (error() || !m_running) {
return; return;
} }