[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kdevelop-devel
Subject:    Re: Killability of ImportProjectJob
From:       Milian Wolff <mail () milianw ! de>
Date:       2010-12-22 23:08:02
Message-ID: 201012230008.05420.mail () milianw ! de
[Download RAW message or body]

[Attachment #2 (multipart/signed)]

[Attachment #4 (multipart/mixed)]


On Saturday 20 November 2010 18:00:04 Milian Wolff wrote:
> On Saturday 20 November 2010 17:50:31 Milian Wolff wrote:
> > Hey all,
> > 
> > currently the ImportProjectJob is not really interruptable. It has doKill
> > that just waitForFinished which can easily lead to deadlocks nowadays,
> > since we can import stuff from the background bug QMetaType::invokeMethod
> 
> > in the projectmodel then. E.g.:
> btw, I just notice that this is probably only hit when I try this patch:

<snip>

> Which would be an (imo) awesome addition to KDevelop. It didn't used to
> work as items where added from random threads but now that they are always
> added from the UI it just works™.

Attached you find my second attempt, this time with a nested event loop (and 
some other cleanup). The deadlock doesn't occur anymore (of course), but 
instead i get nasty crashes, e.g. in the project model, as if the 
index.internalPointer static cast returned an invalid item 
(ProjectModel::parent()). Or something like this:

Thread 5 (Thread 0x7f9ea2094700 (LWP 26811)):
[KCrash Handler]
#6  0x00007f9ebee84bb1 in KDevelop::ProjectFileItem::setUrl 
(this=0x7f9ea841c110, url=...) at 
/home/milian/projects/kde4/kdevplatform/project/projectmodel.cpp:681
#7  0x00007f9ebee84759 in KDevelop::ProjectFileItem::ProjectFileItem 
(this=0x7f9ea841c110, project=0x3098c20, file=..., parent=0x3009be0) at 
/home/milian/projects/kde4/kdevplatform/project/projectmodel.cpp:623
#8  0x00007f9eac139da0 in CMakeManager::reloadFiles (this=0x27c7130, 
item=0x3009be0) at 
/home/milian/projects/kde4/kdevelop/projectmanagers/cmake/cmakemanager.cpp:889
#9  0x00007f9eac137b99 in CMakeManager::parse (this=0x27c7130, item=0x3009be0) 
at 
/home/milian/projects/kde4/kdevelop/projectmanagers/cmake/cmakemanager.cpp:556
#10 0x00007f9ebee8ca4f in KDevelop::ImportProjectJobPrivate::import 
(this=0x306c580, folder=0x3009be0) at 
/home/milian/projects/kde4/kdevplatform/project/importprojectjob.cpp:62
#11 0x00007f9ebee8d026 in 
QtConcurrent::VoidStoredMemberFunctionPointerCall1<void, 
KDevelop::ImportProjectJobPrivate, KDevelop::ProjectFolderItem*, 
KDevelop::ProjectFolderItem*>::runFunctor (this=0x3088320) at 
/usr/include/QtCore/qtconcurrentstoredfunctioncall.h:426
#12 0x00007f9ebee8c75c in QtConcurrent::RunFunctionTask<void>::run 
(this=0x3088320) at /usr/include/QtCore/qtconcurrentrunbase.h:120
#13 0x00007f9ec1eeeb88 in ?? () from /usr/lib/libQtCore.so.4
#14 0x00007f9ec1ef852e in ?? () from /usr/lib/libQtCore.so.4
#15 0x00007f9ec1c6fc60 in start_thread () from /lib/libpthread.so.0
#16 0x00007f9ec034f7ed in clone () from /lib/libc.so.6
#17 0x0000000000000000 in ?? ()

I'll try to run it through valgrind tomorrow, but maybe you got an idea? Or is 
this a bad idea?

Bye
-- 
Milian Wolff
mail@milianw.de
http://milianw.de

["better_import.t1.patch" (text/x-patch)]

diff --git a/project/importprojectjob.cpp b/project/importprojectjob.cpp
index ea26786..53f0fb3 100644
--- a/project/importprojectjob.cpp
+++ b/project/importprojectjob.cpp
@@ -34,6 +34,7 @@
 #include <interfaces/iprojectcontroller.h>
 #include <QPointer>
 #include <KLocale>
+#include <QEventLoop>
 
 namespace KDevelop
 {
@@ -41,21 +42,31 @@ namespace KDevelop
 class ImportProjectJobPrivate
 {
 public:
-    ImportProjectJobPrivate() : cancel(false) {}
+    ImportProjectJobPrivate()
+        : m_folder(0)
+        , m_importer(0)
+        , m_watcher(0)
+        , cancel(false)
+    {}
     ProjectFolderItem *m_folder;
     IProjectFileManager *m_importer;
     QFutureWatcher<void> *m_watcher;
-    QPointer<IProject> m_project;
-    bool cancel;
+    QAtomicInt cancel;
 
     void import(ProjectFolderItem* folder)
     {
+        qDebug() << Q_FUNC_INFO;
+        if (cancel)
+            return;
+
         QList<KDevelop::ProjectFolderItem*> subFolders = m_importer->parse(folder);
         foreach(KDevelop::ProjectFolderItem* sub, subFolders)
         {
-            if(!cancel)
-                import(sub);
-        }  
+            if(cancel)
+                break;
+
+            import(sub);
+        }
     }
 
 };
@@ -67,9 +78,8 @@ ImportProjectJob::ImportProjectJob(ProjectFolderItem *folder, IProjectFileManage
     
     d->m_importer = importer;
     d->m_folder = folder;
-    d->m_project = folder->project();
     
-    setObjectName(i18n("Project Import: %1", d->m_project->name()));
+    setObjectName(i18n("Project Import: %1", folder->project()->name()));
 }
 
 ImportProjectJob::~ImportProjectJob()
@@ -88,6 +98,7 @@ void ImportProjectJob::start()
 
 void ImportProjectJob::importDone()
 {
+    qDebug() << Q_FUNC_INFO;
     d->m_watcher->deleteLater(); /* Goodbye to the QFutureWatcher */
 
     emitResult();
@@ -95,13 +106,31 @@ void ImportProjectJob::importDone()
 
 bool ImportProjectJob::doKill()
 {
-    d->m_watcher->cancel();
+    Q_ASSERT(!d->cancel); // if this is hit, the nested loop caused havoc
+
     d->cancel=true;
-    
+    d->m_watcher->cancel();
+
     setError(1);
     setErrorText(i18n("Project import canceled."));
-    
-    d->m_watcher->waitForFinished();
+
+    // make sure we don't try to kill this job twice...
+    setCapabilities(NoCapabilities);
+
+    if (!d->m_watcher->isFinished()) {
+        qDebug() << "1" << d->m_watcher->isFinished();
+        QEventLoop loop;
+        qDebug() << "2" << d->m_watcher->isFinished();
+        connect(d->m_watcher, SIGNAL(canceled()),
+                &loop, SLOT(quit()));
+        connect(d->m_watcher, SIGNAL(finished()),
+                &loop, SLOT(quit()));
+        qDebug() << "3" << d->m_watcher->isFinished();
+        loop.exec();
+        qDebug() << "4" << d->m_watcher->isFinished();
+        qDebug() << "finished waiting";
+    }
+
     return true;
 }
 
diff --git a/shell/project.cpp b/shell/project.cpp
index 7b2bdf6..244fcec 100644
--- a/shell/project.cpp
+++ b/shell/project.cpp
@@ -237,9 +237,10 @@ public:
         
         loading=false;
         if(job->errorText().isEmpty()) {
-            projCtrl->projectModel()->appendRow(topItem);
             projCtrl->projectImportingFinished( project );
         } else {
+            Q_ASSERT(topItem->index().isValid());
+            projCtrl->projectModel()->removeRow(topItem->row());
             projCtrl->closeProject(project);
         }
     }
@@ -515,6 +516,7 @@ bool Project::open( const KUrl& projectFileUrl_ )
     d->loadVersionControlPlugin(projectGroup);
     d->progress->setBuzzy();
     KJob* importJob = iface->createImportJob(d->topItem );
+    Core::self()->projectController()->projectModel()->appendRow(d->topItem);
     connect( importJob, SIGNAL( result( KJob* ) ), this, SLOT( importDone( KJob* ) ) );
     Core::self()->runController()->registerJob( importJob );
     return true;

["signature.asc" (application/pgp-signature)]

-- 
KDevelop-devel mailing list
KDevelop-devel@kdevelop.org
https://barney.cs.uni-potsdam.de/mailman/listinfo/kdevelop-devel


[prev in list] [next in list] [prev in thread] [next in thread] 

Configure | About | News | Add a list | Sponsored by KoreLogic