[prev in list] [next in list] [prev in thread] [next in thread]
List: kdevelop-devel
Subject: Re: Killability of ImportProjectJob
From: Olivier JG <olivier.jg () gmail ! com>
Date: 2010-12-17 4:09:44
Message-ID: 4D0AE288.9070902 () gmail ! com
[Download RAW message or body]
[Attachment #2 (multipart/alternative)]
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
> <mailto: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 <mailto: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
[Attachment #5 (text/html)]
<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.01 Transitional//EN">
<html>
<head>
<meta content="text/html; charset=UTF-8" http-equiv="Content-Type">
</head>
<body text="#000000" bgcolor="#ffffff">
On 12/17/2010 11:39 AM, Aleix Pol wrote:
<blockquote
cite="mid:AANLkTi=tHFoGue69odvku-=xs-CRgkCh9Qd_VYxm5MOu@mail.gmail.com"
type="cite">
<div class="gmail_quote">On Fri, Dec 17, 2010 at 4:12 AM, Olivier
JG <span dir="ltr"><<a moz-do-not-send="true"
href="mailto:olivier.jg@gmail.com">olivier.jg@gmail.com</a>></span>
wrote:<br>
<blockquote class="gmail_quote" style="margin: 0pt 0pt 0pt
0.8ex; border-left: 1px solid rgb(204, 204, 204);
padding-left: 1ex;">
<div>
<div class="h5">On 11/21/2010 01:00 AM, Milian Wolff wrote:<br>
<blockquote class="gmail_quote" style="margin: 0pt 0pt 0pt
0.8ex; border-left: 1px solid rgb(204, 204, 204);
padding-left: 1ex;">
On Saturday 20 November 2010 17:50:31 Milian Wolff
wrote:<br>
<blockquote class="gmail_quote" style="margin: 0pt 0pt
0pt 0.8ex; border-left: 1px solid rgb(204, 204, 204);
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 moz-do-not-send="true"
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 moz-do-not-send="true"
href="mailto:KDevelop-devel@kdevelop.org" \
target="_blank">KDevelop-devel@kdevelop.org</a><br> <a moz-do-not-send="true"
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>
</blockquote>
<br>
Sure, but from what I can tell, it's never cancelled, because that's
not supported for something started with QtConcurrent::run().<br>
-Olivier JG<br>
<br>
</body>
</html>
--
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