Skip to content
Commit ea81d8fe authored by Igor Kushnir's avatar Igor Kushnir
Browse files

FileManagerListJob: improve killing behavior

AbstractFileManagerPlugin::createImportJob() returns a
FileManagerListJob. CMakeManager::createImportJob() adds the returned
FileManagerListJob as an ExecuteCompositeJob's subjob. The
ExecuteCompositeJob is then registered in RunController. RunController
kills registered jobs sometimes. This killing propagates to the subjobs.

FileManagerListJob does not override doKill(). The base class's KIO::Job
implementation of doKill() kills and clears all subjobs. However,
FileManagerListJob never adds subjobs to the KIO::Job's list, but
creates and starts subjobs one by one instead. So the only useful things
inherited from KIO::Job are the Killable capability and that doKill()
returns true. As doKill() does not abort the local-folder thread
alternative to a subjob, ~FileManagerListJob() ends up aborting and
waiting for the worker thread to finish.

FileManagerListJob sets itself as the parent job of its remote-folder
subjobs. A parent job must inherit KIO::Job. I have analyzed parent job
usages in KIO and found no way, in which the removal of this parent job
can affect KDevelop behavior. Setting the parent job was introduced in
099064e9 without explanation and likely
needlessly (just in case).

Improvements in this commit:
* Inherit KJob instead of KIO::Job, set the Killable capability in
  FileManagerListJob() and implement FileManagerListJob::doKill() more
  efficiently by canceling the local-folder worker thread or killing the
  remote-folder subjob. This requires storing the currently running
  remote folder subjob (if any) and thus connecting to KJob::finished
  instead of KJob::result to ensure no dangling pointers.
* Remove FileManagerListJob::abort() API and use KJob::kill() instead.
* The name "abort" could be confused with std::abort(). Use less
  ambiguous "cancel"-based member names instead.
* Clarify slot names.
* Change argument and return types from KIO::Job* to KJob* as
  KIO::Job's custom API is unused. Don't register the KIO::Job* meta
  type anymore.
* Load and store std::atomic<bool> with relaxed ordering instead of
  QAtomicInt with sequentially consistent memory ordering.
parent 3d69b908
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment