Commit 19d9907d authored by Milian Wolff's avatar Milian Wolff
Browse files

fix crash when files are added/removed by external program

This could 'easily' reproduced by opening a large git-based project in
kdevelop with one of the AbstractFileManager-inheriting project
manager. Then switching between branches resulted in a crash sooner or later.

The main culprit was that FileManagerListJob::slotResult was sometimes
called after a the job was killed. KJob::kill apparently does *not*
kill the job directly, hence we need some additional safeguard here.

Furthermore this patch adds some more safety measures against invalid
item pointers in the FileManagerListJob's m_listQueue.

Please test!

BUG: 260741
parent 860d46f8
......@@ -106,7 +106,7 @@ void AbstractFileManagerPlugin::Private::projectClosing(IProject* project)
// see also addLotsOfFiles test
foreach( FileManagerListJob* job, m_projectJobs[project] ) {
kDebug(9517) << "killing project job:" << job;
job->kill();
job->abort();
}
m_projectJobs.remove(project);
}
......@@ -135,7 +135,7 @@ void AbstractFileManagerPlugin::Private::jobFinished(KJob* job)
{
FileManagerListJob* gmlJob = qobject_cast<FileManagerListJob*>(job);
Q_ASSERT(gmlJob);
ifDebug(kDebug() << job << gmlJob;)
ifDebug(kDebug(9517) << job << gmlJob << gmlJob->item();)
m_projectJobs[ gmlJob->item()->project() ].removeOne( gmlJob );
}
......@@ -361,13 +361,27 @@ void AbstractFileManagerPlugin::Private::continueWatcher(ProjectFolderItem* fold
m_watchers[folder->project()]->restartDirScan(folder->url().toLocalFile());
}
bool isChildItem(ProjectBaseItem* parent, ProjectBaseItem* child)
{
do {
if (child == parent) {
return true;
}
child = child->parent();
} while(child);
return false;
}
void AbstractFileManagerPlugin::Private::removeFolder(ProjectFolderItem* folder)
{
ifDebug(kDebug(9517) << "removing folder:" << folder << folder->url();)
foreach(FileManagerListJob* job, m_projectJobs[folder->project()]) {
if (job->item() == folder) {
if (isChildItem(folder, job->item())) {
kDebug(9517) << "killing list job for removed folder" << job << folder->url();
job->kill();
job->abort();
Q_ASSERT(!m_projectJobs.value(folder->project()).contains(job));
} else {
job->removeSubDir(folder);
}
}
folder->parent()->removeRow( folder->row() );
......
......@@ -27,7 +27,7 @@
using namespace KDevelop;
FileManagerListJob::FileManagerListJob(ProjectFolderItem* item, const bool forceRecursion)
: KIO::Job(), m_item(0), m_forceRecursion(forceRecursion)
: KIO::Job(), m_item(0), m_forceRecursion(forceRecursion), m_aborted(false)
{
/* the following line is not an error in judgement, apparently starting a
* listJob while the previous one hasn't self-destructed takes a lot of time,
......@@ -50,6 +50,11 @@ void FileManagerListJob::addSubDir( ProjectFolderItem* item )
m_listQueue.enqueue(item);
}
void FileManagerListJob::removeSubDir(ProjectFolderItem* item)
{
m_listQueue.removeAll(item);
}
void FileManagerListJob::slotEntries(KIO::Job* job, const KIO::UDSEntryList& entriesIn)
{
Q_UNUSED(job);
......@@ -58,7 +63,7 @@ void FileManagerListJob::slotEntries(KIO::Job* job, const KIO::UDSEntryList& ent
void FileManagerListJob::startNextJob()
{
if ( m_listQueue.isEmpty() ) {
if ( m_listQueue.isEmpty() || m_aborted ) {
return;
}
......@@ -72,6 +77,10 @@ void FileManagerListJob::startNextJob()
void FileManagerListJob::slotResult(KJob* job)
{
if (m_aborted) {
return;
}
emit entries(this, m_item, entryList, m_forceRecursion);
entryList.clear();
......@@ -86,4 +95,13 @@ void FileManagerListJob::slotResult(KJob* job)
}
}
void FileManagerListJob::abort()
{
bool killed = kill();
Q_ASSERT(killed);
Q_UNUSED(killed);
m_aborted = true;
}
#include "filemanagerlistjob.moc"
......@@ -36,6 +36,9 @@ public:
ProjectFolderItem* item() const;
void addSubDir(ProjectFolderItem* item);
void removeSubDir(ProjectFolderItem* item);
void abort();
signals:
void entries(FileManagerListJob* job, ProjectFolderItem* baseItem,
......@@ -53,6 +56,8 @@ private:
ProjectFolderItem* m_item;
KIO::UDSEntryList entryList;
const bool m_forceRecursion;
// kill does not delete the job instantaniously
bool m_aborted;
};
}
......
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