Commit 39fcfac8 authored by Dmitry Kazakov's avatar Dmitry Kazakov
Browse files

Fix Krita discarding native threads every couple of seconds

For years Krita has been spawning native threads carelessly. It
happened because of the implementation of QThreadPool::waitForDone(),
which discards threads on every successful call.

We once had a patch for that (9c34fef3),
but it has been reverted due to deadlock regressions it introduced.

This patch adds a bit different implementation of the waiting strategy,
so (I hope) it won't cause any deadlocks.

BUG:367901
BUG:360677
parent bafbf8ef
......@@ -17,6 +17,7 @@
#include "kis_base_rects_walker.h"
#include "kis_async_merger.h"
#include "kis_updater_context.h"
#include <KoAlwaysInline.h>
//#define DEBUG_JOBS_SEQUENCE
......@@ -46,6 +47,16 @@ public:
}
void run() override {
runImpl();
// notify that the job is exiting and wake everybody
// waiting on wakeForDone()
m_updaterContext->jobThreadExited();
}
private:
ALWAYS_INLINE void runImpl() {
if (!isRunning()) return;
/**
......@@ -109,6 +120,8 @@ public:
}
}
public:
inline void runMergeJob() {
KIS_SAFE_ASSERT_RECOVER_RETURN(m_atomicType == Type::MERGE);
KIS_SAFE_ASSERT_RECOVER_RETURN(m_walker);
......
......@@ -117,6 +117,16 @@ bool KisUpdaterContext::isJobAllowed(KisBaseRectsWalkerSP walker)
return !intersects;
}
void KisUpdaterContext::startThread(int index)
{
{
QMutexLocker l(&m_runningThreadsMutex);
m_numRunningThreads++;
}
m_threadPool.start(m_jobs[index]);
}
/**
* NOTE: In theory, isJobAllowed() and addMergeJob() should be merged into
* one atomic method like `bool push()`, because this implementation
......@@ -136,7 +146,7 @@ void KisUpdaterContext::addMergeJob(KisBaseRectsWalkerSP walker)
// it might happen that we call this function from within
// the thread itself, right when it finished its work
if (shouldStartThread && !m_testingMode) {
m_threadPool.start(m_jobs[jobIndex]);
startThread(jobIndex);
}
}
......@@ -151,7 +161,7 @@ void KisUpdaterContext::addStrokeJob(KisStrokeJob *strokeJob)
// it might happen that we call this function from within
// the thread itself, right when it finished its work
if (shouldStartThread && !m_testingMode) {
m_threadPool.start(m_jobs[jobIndex]);
startThread(jobIndex);
}
}
......@@ -166,13 +176,17 @@ void KisUpdaterContext::addSpontaneousJob(KisSpontaneousJob *spontaneousJob)
// it might happen that we call this function from within
// the thread itself, right when it finished its work
if (shouldStartThread && !m_testingMode) {
m_threadPool.start(m_jobs[jobIndex]);
startThread(jobIndex);
}
}
void KisUpdaterContext::waitForDone()
{
m_threadPool.waitForDone();
QMutexLocker l(&m_runningThreadsMutex);
while(m_numRunningThreads > 0) {
m_waitForDoneCondition.wait(l.mutex());
}
}
bool KisUpdaterContext::walkerIntersectsJob(KisBaseRectsWalkerSP walker,
......@@ -243,6 +257,17 @@ void KisUpdaterContext::jobFinished()
if (m_scheduler) m_scheduler->spareThreadAppeared();
}
void KisUpdaterContext::jobThreadExited()
{
QMutexLocker l(&m_runningThreadsMutex);
m_numRunningThreads--;
KIS_SAFE_ASSERT_RECOVER_NOOP(m_numRunningThreads >= 0);
if (m_numRunningThreads <= 0) {
m_waitForDoneCondition.wakeAll();
}
}
void KisUpdaterContext::setTestingMode(bool value)
{
m_testingMode = value;
......
......@@ -10,6 +10,7 @@
#include <QMutex>
#include <QReadWriteLock>
#include <QThreadPool>
#include <QWaitCondition>
#include "kis_base_rects_walker.h"
#include "kis_async_merger.h"
......@@ -129,6 +130,7 @@ public:
void continueUpdate(const QRect& rc);
void doSomeUsefulWork();
void jobFinished();
void jobThreadExited();
void setTestingMode(bool value);
......@@ -147,6 +149,9 @@ protected:
QReadWriteLock m_exclusiveJobLock;
QMutex m_lock;
QMutex m_runningThreadsMutex;
int m_numRunningThreads = 0;
QWaitCondition m_waitForDoneCondition;
QVector<KisUpdateJobItem*> m_jobs;
QThreadPool m_threadPool;
KisLockFreeLodCounter m_lodCounter;
......@@ -164,6 +169,8 @@ private:
const QVector<KisUpdateJobItem*> getJobs();
void clear();
void startThread(int index);
};
class KRITAIMAGE_EXPORT KisTestableUpdaterContext : public KisUpdaterContext
......
Supports Markdown
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