Skip to content

KJob: add protected isFinished(); emit result() and finished() at most once

Igor Kushnir requested to merge work/kjob-check-is-finished into master

I don't have much experience with KJob and don't know how it is used across the KDE code base. In this merge request I have implemented the features that I miss while fixing potential bugs in KDevelop's KJob subclasses.

I was tempted to use KJob::error() as an imperfect alternative to exposing isFinished to KJob subclasses, but the documentation restricts its usage to Only call this method from the slot connected to result(). So without access to isFinished, when a subclass cannot infer whether it was killed by examining the existing data members, it must resort to adding its own isFinished data member, which means additional error-prone code plus one byte space waste in multiple KJob subclasses.

Guarding against duplicate signal emissions in each KJob subclass is a lot of boilerplate. Enforcing this on the KJob level is much more reliable and requires a lot less code.

I have managed to reproduce duplicate KJob::finished() and KJob::result() signals by calling KJob::kill() twice in a row or calling start() and kill(). For example, I replaced in KJobTest::testKill() job->kill(static_cast<KJob::KillVerbosity>(killVerbosity)); with

    QTimer::singleShot(0, job, [=] {
        job->kill(static_cast<KJob::KillVerbosity>(killVerbosity));
    });
    job->start();

The output without my fixes is:

FAIL!  : KJobTest::testKill(killed with result) Compared values are not the same
   Actual   (m_resultCount)  : 2
   Expected (resultEmitCount): 1
   Loc: [../autotests/kjobtest.cpp(255)]
FAIL!  : KJobTest::testKill(killed quietly) Compared values are not the same
   Actual   (m_resultCount)  : 1
   Expected (resultEmitCount): 0
   Loc: [../autotests/kjobtest.cpp(255)]
Edited by Igor Kushnir

Merge request reports