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
Please register or sign in to comment