From kdevelop-devel Fri Dec 17 04:14:01 2010 From: Aleix Pol Date: Fri, 17 Dec 2010 04:14:01 +0000 To: kdevelop-devel Subject: Re: Killability of ImportProjectJob Message-Id: X-MARC-Message: https://marc.info/?l=kdevelop-devel&m=129255935519173 MIME-Version: 1 Content-Type: multipart/mixed; boundary="--===============0897893739==" --===============0897893739== Content-Type: multipart/alternative; boundary=0015175cb6f2764b6a0497936687 --0015175cb6f2764b6a0497936687 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable It works for cmake, if nobody changed it from back when I implemented it, a= t least. Aleix On Fri, Dec 17, 2010 at 5:09 AM, Olivier JG wrote: > On 12/17/2010 11:39 AM, Aleix Pol wrote: > > On Fri, Dec 17, 2010 at 4:12 AM, Olivier JG wrote: > >> On 11/21/2010 01:00 AM, 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= : >>> >>> diff --git a/shell/project.cpp b/shell/project.cpp >>> index c8fbc53..877c4a9 100644 >>> --- a/shell/project.cpp >>> +++ b/shell/project.cpp >>> @@ -237,9 +237,10 @@ public: >>> >>> loading=3Dfalse; >>> 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); >>> } >>> } >>> @@ -516,6 +517,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; >>> >>> 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 >>> >> >> As I noted here: https://bugs.kde.org/show_bug.cgi?id=3D256709 it seems >> that the ImportProjectJob cannot be canceled because it is run with >> QtConcurrent::run(), which the documentation states doesn't support bein= g >> canceled. All you can do is wait for it to finish. >> Until such a time as someone rewrites ProjectImportJob to support killin= g, >> shouldn't the ImportProjectJob simply not advertise itself as Killable? >> >> (As a side note, when I remove the call to setCapabilities(killable), th= e >> stop button still gets enabled, but simply doesn't do anything...) >> >> -Olivier JG >> >> -- >> KDevelop-devel mailing list >> KDevelop-devel@kdevelop.org >> https://barney.cs.uni-potsdam.de/mailman/listinfo/kdevelop-devel >> > > Right now what we're doing is that if it's cancelled the process is told = to > be finished ASAP. If the project is using the "QList ::parse()" method > properly it will stop reading further in the tree and close the project. > > Aleix > > > Sure, but from what I can tell, it's never cancelled, because that's not > supported for something started with QtConcurrent::run(). > -Olivier JG > > > -- > KDevelop-devel mailing list > KDevelop-devel@kdevelop.org > https://barney.cs.uni-potsdam.de/mailman/listinfo/kdevelop-devel > > --0015175cb6f2764b6a0497936687 Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: quoted-printable It works for cmake, if nobody changed it from back when I implemented it, a= t least.

Aleix

On Fri,= Dec 17, 2010 at 5:09 AM, Olivier JG <olivier.jg@gmail.com> wrote:
=20 =20 =20
On 12/17/2010 11:39 AM, Aleix Pol wrote:
On Fri, Dec 17, 2010 at 4:12 AM, Olivier JG <olivier.jg@gmail.com> wrote:
On 11/21/2010 01:00 AM, 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:

diff --git a/shell/project.cpp b/shell/project.cpp
index c8fbc53..877c4a9 100644
--- a/shell/project.cpp
+++ b/shell/project.cpp
@@ -237,9 +237,10 @@ public:

=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0loading=3Dfalse;
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0if(job->errorText().is= Empty()) {
- =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0projCtrl->projectModel()->appendRow(topItem); =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0projCtrl-&g= t;projectImportingFinished( project );
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0} else {
+ =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0Q_ASSERT(topItem= ->index().isValid());
+ =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0projCtrl->projectModel()->removeRow(topItem->= ;row());
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0projCtrl-&g= t;closeProject(project);
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0}
=C2=A0 =C2=A0 =C2=A0}
@@ -516,6 +517,7 @@ bool Project::open( const KUrl& =C2=A0projectFileUrl_ )
=C2=A0 =C2=A0 =C2=A0d->loadVersionControlPlugin(projectG= roup);
=C2=A0 =C2=A0 =C2=A0d->progress->setBuzzy();
=C2=A0 =C2=A0 =C2=A0KJob* importJob =3D iface->createImportJob(d->topItem );
+ =C2=A0 =C2=A0Core::self()->projectController()->projectModel()->appendRow= (d->topItem);
=C2=A0 =C2=A0 =C2=A0connect( importJob, SIGNAL( result( KJo= b* ) ), this, SLOT( importDone(
KJob* ) ) );
=C2=A0 =C2=A0 =C2=A0Core::self()->runController()->re= gisterJob( importJob );
=C2=A0 =C2=A0 =C2=A0return true;

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

As I noted here: https://bugs.kde.org/show_bug.cgi?id=3D256709<= /a> it seems that the ImportProjectJob cannot be canceled because it is run with QtConcurrent::run(), which the documentation states doesn't support being canceled. All you can do is wait for it to finish.
Until such a time as someone rewrites ProjectImportJob to support killing, shouldn't the ImportProjectJob simply not advertise itself as Killable?

(As a side note, when I remove the call to setCapabilities(killable), the stop button still gets enabled, but simply doesn't do anything...)

-Olivier JG

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

Right now what we're doing is that if it's cancelled the process is told to be finished ASAP. If the project is using the "QList ::parse()" method properly it will stop reading fu= rther in the tree and close the project.

Aleix

Sure, but from what I can tell, it's never cancelled, because that&= #39;s not supported for something started with QtConcurrent::run().
-Olivier JG


--
KDevelop-devel mailing list
KDevelop-devel@kdevelop.org<= /a>
https://barney.cs.uni-potsdam.de/mailman/listinfo/kdeve= lop-devel


--0015175cb6f2764b6a0497936687-- --===============0897893739== 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 --===============0897893739==--