[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:20:50
Message-ID: 4D0AE522.8060400 () gmail ! com
[Download RAW message or body]

[Attachment #2 (multipart/alternative)]


On 12/17/2010 12:14 PM, Aleix Pol wrote:
> It works for cmake, if nobody changed it from back when I implemented 
> it, at least.
Could you try to reload Kdevelop and press stop? That just hangs every 
time for me, and based on what the Qt docs say, there's no reason for it 
to do anything else.
-Olivier JG
>
> Aleix
>
> On Fri, Dec 17, 2010 at 5:09 AM, Olivier JG <olivier.jg@gmail.com 
> <mailto:olivier.jg@gmail.com>> wrote:
>
>     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
>
>
>     --
>     KDevelop-devel mailing list
>     KDevelop-devel@kdevelop.org <mailto:KDevelop-devel@kdevelop.org>
>     https://barney.cs.uni-potsdam.de/mailman/listinfo/kdevelop-devel
>
>


[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 12:14 PM, Aleix Pol wrote:
    <blockquote
      cite="mid:AANLkTi=K25VqGwaRHQ5uAPOHMLb1yU_HziBwDi04xHNY@mail.gmail.com"
      type="cite">It works for cmake, if nobody changed it from back
      when I implemented it, at least.</blockquote>
    Could you try to reload Kdevelop and press stop? That just hangs
    every time for me, and based on what the Qt docs say, there's no
    reason for it to do anything else.<br>
    -Olivier JG<br>
    <blockquote
      cite="mid:AANLkTi=K25VqGwaRHQ5uAPOHMLb1yU_HziBwDi04xHNY@mail.gmail.com"
      type="cite">
      <div><br>
      </div>
      <div>Aleix<br>
        <br>
        <div class="gmail_quote">On Fri, Dec 17, 2010 at 5:09 AM,
          Olivier JG <span dir="ltr">&lt;<a moz-do-not-send="true"
              href="mailto:olivier.jg@gmail.com">olivier.jg@gmail.com</a>&gt;</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 text="#000000" bgcolor="#ffffff">
              <div>
                <div class="h5"> On 12/17/2010 11:39 AM, Aleix Pol
                  wrote:
                  <blockquote type="cite">
                    <div class="gmail_quote">On Fri, Dec 17, 2010 at
                      4:12 AM, Olivier JG <span dir="ltr">&lt;<a
                          moz-do-not-send="true"
                          href="mailto:olivier.jg@gmail.com"
                          target="_blank">olivier.jg@gmail.com</a>&gt;</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>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-&gt;errorText().isEmpty())
                              {<br>
                              -          
                               \
projCtrl-&gt;projectModel()-&gt;appendRow(topItem);<br>  
                               projCtrl-&gt;projectImportingFinished(
                              project );<br>
                                       } else {<br>
                              +          
                               Q_ASSERT(topItem-&gt;index().isValid());<br>
                              +          
                               \
projCtrl-&gt;projectModel()-&gt;removeRow(topItem-&gt;row());<br>  
                               projCtrl-&gt;closeProject(project);<br>
                                       }<br>
                                   }<br>
                              @@ -516,6 +517,7 @@ bool Project::open(
                              const KUrl&amp;  projectFileUrl_ )<br>
                                 
                               d-&gt;loadVersionControlPlugin(projectGroup);<br>
                                   d-&gt;progress-&gt;setBuzzy();<br>
                                   KJob* importJob =
                              iface-&gt;createImportJob(d-&gt;topItem );<br>
                              +  
 Core::self()-&gt;projectController()-&gt;projectModel()-&gt;appendRow(d-&gt;topItem);<br>
  connect( importJob, SIGNAL( result(
                              KJob* ) ), this, SLOT( importDone(<br>
                              KJob* ) ) );<br>
                                 
                               Core::self()-&gt;runController()-&gt;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>
                </div>
              </div>
              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>
            </div>
            <br>
            --<br>
            KDevelop-devel mailing list<br>
            <a moz-do-not-send="true"
              href="mailto:KDevelop-devel@kdevelop.org">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>
  <br>
          </blockquote>
        </div>
        <br>
      </div>
    </blockquote>
    <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