diff --git a/plugins/subversion/svnblamejob.cpp b/plugins/subversion/svnblamejob.cpp --- a/plugins/subversion/svnblamejob.cpp +++ b/plugins/subversion/svnblamejob.cpp @@ -143,7 +143,7 @@ : SvnJobBaseImpl( parent, KDevelop::OutputJob::Silent ) { setType( KDevelop::VcsJob::Annotate ); - connect(m_job, &SvnInternalBlameJob::blameLine, + connect(m_job.data(), &SvnInternalBlameJob::blameLine, this, &SvnBlameJob::blameLineReceived); setObjectName(i18n("Subversion Annotate")); } diff --git a/plugins/subversion/svndiffjob.cpp b/plugins/subversion/svndiffjob.cpp --- a/plugins/subversion/svndiffjob.cpp +++ b/plugins/subversion/svndiffjob.cpp @@ -258,7 +258,7 @@ : SvnJobBaseImpl( parent, KDevelop::OutputJob::Silent ) { setType( KDevelop::VcsJob::Add ); - connect( m_job, &SvnInternalDiffJob::gotDiff, + connect( m_job.data(), &SvnInternalDiffJob::gotDiff, this, &SvnDiffJob::setDiff, Qt::QueuedConnection ); setObjectName(i18n("Subversion Diff")); diff --git a/plugins/subversion/svninfojob.cpp b/plugins/subversion/svninfojob.cpp --- a/plugins/subversion/svninfojob.cpp +++ b/plugins/subversion/svninfojob.cpp @@ -90,7 +90,7 @@ : SvnJobBaseImpl( parent, KDevelop::OutputJob::Silent ), m_provideInfo( SvnInfoJob::AllInfo ) { setType( KDevelop::VcsJob::Add ); - connect( m_job, &SvnInternalInfoJob::gotInfo, + connect( m_job.data(), &SvnInternalInfoJob::gotInfo, this, &SvnInfoJob::setInfo, Qt::QueuedConnection ); setObjectName(i18n("Subversion Info")); } diff --git a/plugins/subversion/svninternaljobbase.h b/plugins/subversion/svninternaljobbase.h --- a/plugins/subversion/svninternaljobbase.h +++ b/plugins/subversion/svninternaljobbase.h @@ -49,7 +49,7 @@ { Q_OBJECT public: - explicit SvnInternalJobBase( SvnJobBase* parent = nullptr ); + explicit SvnInternalJobBase(SvnJobBase* parentJob); ~SvnInternalJobBase() override; bool success() const override; @@ -120,6 +120,7 @@ bool sendFirstDelta = false; bool killed = false; QString m_errorMessage; + SvnJobBase* m_parentJob; }; diff --git a/plugins/subversion/svninternaljobbase.cpp b/plugins/subversion/svninternaljobbase.cpp --- a/plugins/subversion/svninternaljobbase.cpp +++ b/plugins/subversion/svninternaljobbase.cpp @@ -39,12 +39,12 @@ #include "kdevsvncpp/apr.hpp" #include "kdevsvncpp/revision.hpp" -SvnInternalJobBase::SvnInternalJobBase( SvnJobBase* parent ) - : QObject(parent) - , m_ctxt( new svn::Context() ) +SvnInternalJobBase::SvnInternalJobBase(SvnJobBase* parentJob) + : m_ctxt( new svn::Context() ) , m_guiSemaphore( 0 ) , m_mutex() , m_killMutex() + , m_parentJob(parentJob) { m_ctxt->setListener(this); } @@ -69,6 +69,8 @@ emit failed(); } emit done(); + // at this ppint this object cannot yet be deleted (e.g. as part of the parent job destruction, + // ThreadWeaver logic still holds and uses a reference to finish the execution logic } bool SvnInternalJobBase::contextGetLogin( const std::string& realm, @@ -215,19 +217,18 @@ void SvnInternalJobBase::initBeforeRun() { - auto parentJob = static_cast(parent()); connect( this, &SvnInternalJobBase::needCommitMessage, - parentJob, &SvnJobBase::askForCommitMessage, Qt::QueuedConnection ); + m_parentJob, &SvnJobBase::askForCommitMessage, Qt::QueuedConnection ); connect( this, &SvnInternalJobBase::needLogin, - parentJob, &SvnJobBase::askForLogin, Qt::QueuedConnection ); + m_parentJob, &SvnJobBase::askForLogin, Qt::QueuedConnection ); connect( this, &SvnInternalJobBase::needSslServerTrust, - parentJob, &SvnJobBase::askForSslServerTrust, Qt::QueuedConnection ); + m_parentJob, &SvnJobBase::askForSslServerTrust, Qt::QueuedConnection ); connect( this, &SvnInternalJobBase::showNotification, - parentJob, &SvnJobBase::showNotification, Qt::QueuedConnection ); + m_parentJob, &SvnJobBase::showNotification, Qt::QueuedConnection ); connect( this, &SvnInternalJobBase::needSslClientCert, - parentJob, &SvnJobBase::askForSslClientCert, Qt::QueuedConnection ); + m_parentJob, &SvnJobBase::askForSslClientCert, Qt::QueuedConnection ); connect( this, &SvnInternalJobBase::needSslClientCertPassword, - parentJob, &SvnJobBase::askForSslClientCertPassword, Qt::QueuedConnection ); + m_parentJob, &SvnJobBase::askForSslClientCertPassword, Qt::QueuedConnection ); } svn::ContextListener::SslServerTrustAnswer SvnInternalJobBase::contextSslServerTrustPrompt( diff --git a/plugins/subversion/svnjobbase.h b/plugins/subversion/svnjobbase.h --- a/plugins/subversion/svnjobbase.h +++ b/plugins/subversion/svnjobbase.h @@ -18,13 +18,15 @@ #include "debug.h" #include +#include extern "C" { #include } class SvnInternalJobBase; +using SvnInternalJobBasePtr = QSharedPointer; namespace ThreadWeaver { @@ -39,7 +41,7 @@ public: explicit SvnJobBase( KDevSvnPlugin*, KDevelop::OutputJob::OutputJobVerbosity verbosity = KDevelop::OutputJob::Verbose ); ~SvnJobBase() override; - virtual SvnInternalJobBase* internalJob() const = 0; + virtual SvnInternalJobBasePtr internalJob() const = 0; KDevelop::VcsJob::JobStatus status() const override; KDevelop::IPlugin* vcsPlugin() const override; @@ -74,17 +76,17 @@ explicit SvnJobBaseImpl(KDevSvnPlugin* plugin, KDevelop::OutputJob::OutputJobVerbosity verbosity = KDevelop::OutputJob::Verbose) : SvnJobBase(plugin, verbosity) + , m_job(new InternalJobClass(this)) { - m_job = new InternalJobClass(this); } - SvnInternalJobBase* internalJob() const override + SvnInternalJobBasePtr internalJob() const override { return m_job; } protected: - InternalJobClass* m_job = nullptr; + QSharedPointer m_job; }; #endif diff --git a/plugins/subversion/svnjobbase.cpp b/plugins/subversion/svnjobbase.cpp --- a/plugins/subversion/svnjobbase.cpp +++ b/plugins/subversion/svnjobbase.cpp @@ -41,13 +41,21 @@ void SvnJobBase::startInternalJob() { auto job = internalJob(); - connect( job, &SvnInternalJobBase::failed, + connect( job.data(), &SvnInternalJobBase::failed, this, &SvnJobBase::internalJobFailed, Qt::QueuedConnection ); - connect( job, &SvnInternalJobBase::done, + connect( job.data(), &SvnInternalJobBase::done, this, &SvnJobBase::internalJobDone, Qt::QueuedConnection ); - connect( job, &SvnInternalJobBase::started, + connect( job.data(), &SvnInternalJobBase::started, this, &SvnJobBase::internalJobStarted, Qt::QueuedConnection ); - m_part->jobQueue()->stream() << ThreadWeaver::make_job_raw(job); + // add as shared pointer + // the signals "done" & "failed" are emitted when the queue and the executor still + // have and use a reference to the job, in the execution thread. + // As the this parent job will be deleted in the main/other thread + // (due to deleteLater() being called on it in the KJob::exec()) + // and the ThreadWeaver queue will release the last reference to the passed + // JobInterface pointer only after the JobInterface::execute() method has been left, + // the internal threaded job thus needs to get shared memory management via the QSharedPointer. + m_part->jobQueue()->stream() << job; } bool SvnJobBase::doKill() @@ -133,7 +141,7 @@ void SvnJobBase::internalJobStarted() { - qCDebug(PLUGIN_SVN) << "job started" << static_cast(internalJob()); + qCDebug(PLUGIN_SVN) << "job started" << static_cast(internalJob().data()); m_status = KDevelop::VcsJob::JobRunning; } diff --git a/plugins/subversion/svnlogjob.cpp b/plugins/subversion/svnlogjob.cpp --- a/plugins/subversion/svnlogjob.cpp +++ b/plugins/subversion/svnlogjob.cpp @@ -113,7 +113,7 @@ : SvnJobBaseImpl( parent, KDevelop::OutputJob::Silent ) { setType( KDevelop::VcsJob::Log ); - connect( m_job, &SvnInternalLogJob::logEvent, + connect( m_job.data(), &SvnInternalLogJob::logEvent, this, &SvnLogJob::logEventReceived ); setObjectName(i18n("Subversion Log")); diff --git a/plugins/subversion/svnstatusjob.cpp b/plugins/subversion/svnstatusjob.cpp --- a/plugins/subversion/svnstatusjob.cpp +++ b/plugins/subversion/svnstatusjob.cpp @@ -128,7 +128,7 @@ : SvnJobBaseImpl( parent, KDevelop::OutputJob::Silent ) { setType( KDevelop::VcsJob::Status ); - connect(m_job, &SvnInternalStatusJob::gotNewStatus, + connect(m_job.data(), &SvnInternalStatusJob::gotNewStatus, this, &SvnStatusJob::addToStats, Qt::QueuedConnection); setObjectName(i18n("Subversion Status")); }