From kdevelop-devel Wed Dec 22 23:08:02 2010 From: Milian Wolff Date: Wed, 22 Dec 2010 23:08:02 +0000 To: kdevelop-devel Subject: Re: Killability of ImportProjectJob Message-Id: <201012230008.05420.mail () milianw ! de> X-MARC-Message: https://marc.info/?l=kdevelop-devel&m=129305936906583 MIME-Version: 1 Content-Type: multipart/mixed; boundary="--===============1516507504==" --===============1516507504== Content-Type: multipart/signed; boundary="nextPart1327240.q2ysJqHcAx"; protocol="application/pgp-signature"; micalg=pgp-sha1 Content-Transfer-Encoding: 7bit --nextPart1327240.q2ysJqHcAx Content-Type: multipart/mixed; boundary="Boundary-01=_SToENefX/8D9ul/" Content-Transfer-Encoding: 7bit --Boundary-01=_SToENefX/8D9ul/ Content-Type: Text/Plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable Content-Disposition: inline On Saturday 20 November 2010 18:00:04 Milian Wolff wrote: > On Saturday 20 November 2010 17:50:31 Milian Wolff wrote: > > Hey all, > >=20 > > currently the ImportProjectJob is not really interruptable. It has doKi= ll > > that just waitForFinished which can easily lead to deadlocks nowadays, > > since we can import stuff from the background bug QMetaType::invokeMeth= od >=20 > > in the projectmodel then. E.g.: > btw, I just notice that this is probably only hit when I try this patch: > 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=E2=84=A2. Attached you find my second attempt, this time with a nested event loop (an= d=20 some other cleanup). The deadlock doesn't occur anymore (of course), but=20 instead i get nasty crashes, e.g. in the project model, as if the=20 index.internalPointer static cast returned an invalid item=20 (ProjectModel::parent()). Or something like this: Thread 5 (Thread 0x7f9ea2094700 (LWP 26811)): [KCrash Handler] #6 0x00007f9ebee84bb1 in KDevelop::ProjectFileItem::setUrl=20 (this=3D0x7f9ea841c110, url=3D...) at=20 /home/milian/projects/kde4/kdevplatform/project/projectmodel.cpp:681 #7 0x00007f9ebee84759 in KDevelop::ProjectFileItem::ProjectFileItem=20 (this=3D0x7f9ea841c110, project=3D0x3098c20, file=3D..., parent=3D0x3009be0= ) at=20 /home/milian/projects/kde4/kdevplatform/project/projectmodel.cpp:623 #8 0x00007f9eac139da0 in CMakeManager::reloadFiles (this=3D0x27c7130,=20 item=3D0x3009be0) at=20 /home/milian/projects/kde4/kdevelop/projectmanagers/cmake/cmakemanager.cpp:= 889 #9 0x00007f9eac137b99 in CMakeManager::parse (this=3D0x27c7130, item=3D0x3= 009be0)=20 at=20 /home/milian/projects/kde4/kdevelop/projectmanagers/cmake/cmakemanager.cpp:= 556 #10 0x00007f9ebee8ca4f in KDevelop::ImportProjectJobPrivate::import=20 (this=3D0x306c580, folder=3D0x3009be0) at=20 /home/milian/projects/kde4/kdevplatform/project/importprojectjob.cpp:62 #11 0x00007f9ebee8d026 in=20 QtConcurrent::VoidStoredMemberFunctionPointerCall1::runFunctor (this=3D0x3088320) at=20 /usr/include/QtCore/qtconcurrentstoredfunctioncall.h:426 #12 0x00007f9ebee8c75c in QtConcurrent::RunFunctionTask::run=20 (this=3D0x3088320) 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=20 this a bad idea? Bye =2D-=20 Milian Wolff mail@milianw.de http://milianw.de --Boundary-01=_SToENefX/8D9ul/ Content-Type: text/x-patch; charset="UTF-8"; name="better_import.t1.patch" Content-Transfer-Encoding: quoted-printable Content-Disposition: attachment; filename="better_import.t1.patch" diff --git a/project/importprojectjob.cpp b/project/importprojectjob.cpp index ea26786..53f0fb3 100644 =2D-- a/project/importprojectjob.cpp +++ b/project/importprojectjob.cpp @@ -34,6 +34,7 @@ #include #include #include +#include =20 namespace KDevelop { @@ -41,21 +42,31 @@ namespace KDevelop class ImportProjectJobPrivate { public: =2D ImportProjectJobPrivate() : cancel(false) {} + ImportProjectJobPrivate() + : m_folder(0) + , m_importer(0) + , m_watcher(0) + , cancel(false) + {} ProjectFolderItem *m_folder; IProjectFileManager *m_importer; QFutureWatcher *m_watcher; =2D QPointer m_project; =2D bool cancel; + QAtomicInt cancel; =20 void import(ProjectFolderItem* folder) { + qDebug() << Q_FUNC_INFO; + if (cancel) + return; + QList subFolders =3D m_importer->par= se(folder); foreach(KDevelop::ProjectFolderItem* sub, subFolders) { =2D if(!cancel) =2D import(sub); =2D } =20 + if(cancel) + break; + + import(sub); + } } =20 }; @@ -67,9 +78,8 @@ ImportProjectJob::ImportProjectJob(ProjectFolderItem *fol= der, IProjectFileManage =20 d->m_importer =3D importer; d->m_folder =3D folder; =2D d->m_project =3D folder->project(); =20 =2D setObjectName(i18n("Project Import: %1", d->m_project->name())); + setObjectName(i18n("Project Import: %1", folder->project()->name())); } =20 ImportProjectJob::~ImportProjectJob() @@ -88,6 +98,7 @@ void ImportProjectJob::start() =20 void ImportProjectJob::importDone() { + qDebug() << Q_FUNC_INFO; d->m_watcher->deleteLater(); /* Goodbye to the QFutureWatcher */ =20 emitResult(); @@ -95,13 +106,31 @@ void ImportProjectJob::importDone() =20 bool ImportProjectJob::doKill() { =2D d->m_watcher->cancel(); + Q_ASSERT(!d->cancel); // if this is hit, the nested loop caused havoc + d->cancel=3Dtrue; =2D =20 + d->m_watcher->cancel(); + setError(1); setErrorText(i18n("Project import canceled.")); =2D =20 =2D 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; } =20 diff --git a/shell/project.cpp b/shell/project.cpp index 7b2bdf6..244fcec 100644 =2D-- a/shell/project.cpp +++ b/shell/project.cpp @@ -237,9 +237,10 @@ public: =20 loading=3Dfalse; if(job->errorText().isEmpty()) { =2D 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 =3D iface->createImportJob(d->topItem ); + Core::self()->projectController()->projectModel()->appendRow(d->topIte= m); connect( importJob, SIGNAL( result( KJob* ) ), this, SLOT( importDone(= KJob* ) ) ); Core::self()->runController()->registerJob( importJob ); return true; --Boundary-01=_SToENefX/8D9ul/-- --nextPart1327240.q2ysJqHcAx Content-Type: application/pgp-signature; name=signature.asc Content-Description: This is a digitally signed message part. -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.11 (GNU/Linux) iEYEABECAAYFAk0ShNIACgkQDA6yEs0dE5OCjQCeKRGTl8nByFMaiJyqD+LPbQJz sboAnif32Vy/xEgp/1RoMCKJRCgNZIFO =VRh5 -----END PGP SIGNATURE----- --nextPart1327240.q2ysJqHcAx-- --===============1516507504== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline -- KDevelop-devel mailing list KDevelop-devel@kdevelop.org https://barney.cs.uni-potsdam.de/mailman/listinfo/kdevelop-devel --===============1516507504==--