[prev in list] [next in list] [prev in thread] [next in thread]
List: kdevelop-devel
Subject: Re: Killability of ImportProjectJob
From: Aleix Pol <aleixpol () kde ! org>
Date: 2010-12-17 3:39:53
Message-ID: AANLkTi=tHFoGue69odvku-=xs-CRgkCh9Qd_VYxm5MOu () mail ! gmail ! com
[Download RAW message or body]
[Attachment #2 (multipart/alternative)]
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:
>>
>> 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);
>> }
>> }
>> @@ -516,6 +517,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;
>>
>> 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™
>>
>
> As I noted here: https://bugs.kde.org/show_bug.cgi?id=256709 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/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
[Attachment #5 (text/html)]
<div class="gmail_quote">On Fri, Dec 17, 2010 at 4:12 AM, Olivier JG <span \
dir="ltr"><<a href="mailto:olivier.jg@gmail.com">olivier.jg@gmail.com</a>></span> \
wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px \
#ccc solid;padding-left:1ex;"> <div><div></div><div class="h5">On 11/21/2010 01:00 \
AM, Milian Wolff wrote:<br> <blockquote class="gmail_quote" style="margin:0 0 0 \
.8ex;border-left:1px #ccc solid;padding-left:1ex"> On Saturday 20 November 2010 \
17:50:31 Milian Wolff wrote:<br> <blockquote class="gmail_quote" style="margin:0 0 0 \
.8ex;border-left:1px #ccc solid;padding-left:1ex"> Hey all,<br>
<br>
currently the ImportProjectJob is not really interruptable. It has doKill<br>
that just waitForFinished which can easily lead to deadlocks nowadays,<br>
since we can import stuff from the background bug QMetaType::invokeMethod<br>
in the projectmodel then. E.g.:<br>
</blockquote>
btw, I just notice that this is probably only hit when I try this patch:<br>
<br>
diff --git a/shell/project.cpp b/shell/project.cpp<br>
index c8fbc53..877c4a9 100644<br>
--- a/shell/project.cpp<br>
+++ b/shell/project.cpp<br>
@@ -237,9 +237,10 @@ public:<br>
<br>
loading=false;<br>
if(job->errorText().isEmpty()) {<br>
- projCtrl->projectModel()->appendRow(topItem);<br>
projCtrl->projectImportingFinished( project );<br>
} else {<br>
+ Q_ASSERT(topItem->index().isValid());<br>
+ projCtrl->projectModel()->removeRow(topItem->row());<br>
projCtrl->closeProject(project);<br>
}<br>
}<br>
@@ -516,6 +517,7 @@ bool Project::open( const KUrl& projectFileUrl_ )<br>
d->loadVersionControlPlugin(projectGroup);<br>
d->progress->setBuzzy();<br>
KJob* importJob = iface->createImportJob(d->topItem );<br>
+ Core::self()->projectController()->projectModel()->appendRow(d->topItem);<br>
connect( importJob, SIGNAL( result( KJob* ) ), this, SLOT( importDone(<br>
KJob* ) ) );<br>
Core::self()->runController()->registerJob( importJob );<br>
return true;<br>
<br>
Which would be an (imo) awesome addition to KDevelop. It didn't used to work<br>
as items where added from random threads but now that they are always added<br>
from the UI it just works™<br>
</blockquote>
<br></div></div>
As I noted here: <a href="https://bugs.kde.org/show_bug.cgi?id=256709" \
target="_blank">https://bugs.kde.org/show_bug.cgi?id=256709</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.<br>
Until such a time as someone rewrites ProjectImportJob to support killing, \
shouldn't the ImportProjectJob simply not advertise itself as Killable?<br> <br>
(As a side note, when I remove the call to setCapabilities(killable), the stop button \
still gets enabled, but simply doesn't do anything...)<br> <br>
-Olivier JG<br><font color="#888888">
<br>
-- <br>
KDevelop-devel mailing list<br>
<a href="mailto:KDevelop-devel@kdevelop.org" \
target="_blank">KDevelop-devel@kdevelop.org</a><br> <a \
href="https://barney.cs.uni-potsdam.de/mailman/listinfo/kdevelop-devel" \
target="_blank">https://barney.cs.uni-potsdam.de/mailman/listinfo/kdevelop-devel</a><br>
</font></blockquote></div><br><div>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.</div> <div><br></div><div>Aleix</div>
--
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