Commit 52cd407d authored by Dmitry Kazakov's avatar Dmitry Kazakov

Add an assert and a unittest for a bug in KisTiledExtentManager

The assert should not cause any crashes, only artifacts because
of invalid extents are possible.

NOTE: the bug itself is still present!
parent 5cff2b6f
......@@ -41,7 +41,7 @@ KisTiledExtentManager::Data::~Data()
delete[] m_buffer;
}
inline bool KisTiledExtentManager::Data::add(qint32 index)
bool KisTiledExtentManager::Data::add(qint32 index)
{
QReadLocker lock(&m_migrationLock);
qint32 currentIndex = m_offset + index;
......@@ -79,7 +79,7 @@ inline bool KisTiledExtentManager::Data::add(qint32 index)
return needsUpdateExtent;
}
inline bool KisTiledExtentManager::Data::remove(qint32 index)
bool KisTiledExtentManager::Data::remove(qint32 index)
{
QReadLocker lock(&m_migrationLock);
qint32 currentIndex = m_offset + index;
......@@ -104,7 +104,18 @@ inline bool KisTiledExtentManager::Data::remove(qint32 index)
m_buffer[currentIndex].deref();
}
} else {
m_buffer[currentIndex].deref();
const bool nonZero = m_buffer[currentIndex].deref();
/**
* BUG: since we are not holding the write lock, it might happen
* that two threads failed the original 'if' and entered this
* branch. It means that the device will report unexistent
* extent... See KisTiledDataManagerTest::stressTestExtentsColumn()
* for reproduction.
*
* The same bug is present in KisTiledExtentManager::Data::add()
*/
KIS_SAFE_ASSERT_RECOVER_NOOP(nonZero);
}
return needsUpdateExtent;
......
......@@ -31,14 +31,14 @@ class KRITAIMAGE_EXPORT KisTiledExtentManager
{
static const qint32 InitialBufferSize = 256;
class Data
class KRITAIMAGE_EXPORT Data
{
public:
Data();
~Data();
inline bool add(qint32 index);
inline bool remove(qint32 index);
bool add(qint32 index);
bool remove(qint32 index);
void replace(const QVector<qint32> &indexes);
void clear();
bool isEmpty();
......@@ -76,6 +76,7 @@ public:
private:
void updateExtent();
friend class KisTiledDataManagerTest;
private:
mutable QReadWriteLock m_extentLock;
......
......@@ -942,6 +942,54 @@ void KisTiledDataManagerTest::stressTestLazyCopying()
pool.waitForDone();
}
void KisTiledDataManagerTest::stressTestExtentsColumn()
{
KisTiledExtentManager::Data column;
struct Job : public QRunnable
{
Job(KisTiledExtentManager::Data &column, int index, int numCycles)
: m_column(column), m_index(index), m_numCycles(numCycles) {}
void run() override {
for(qint32 i = 0; i < m_numCycles; i++) {
if (!m_isCreated) {
m_column.add(m_index);
} else {
m_column.remove(m_index);
}
m_isCreated = !m_isCreated;
}
}
KisTiledExtentManager::Data &m_column;
const int m_index;
const int m_numCycles;
bool m_isCreated = false;
};
#ifdef LIMIT_LONG_TESTS
const int numThreads = 8;
const int numWorkers = 16;
const int numCycles = 10000;
#else
const int numThreads = 16;
const int numWorkers = 32;
const int numCycles = 100000;
#endif
QThreadPool pool;
pool.setMaxThreadCount(numThreads);
for(qint32 i = 0; i < numWorkers; i++) {
const int index = 18 + i / 13;
pool.start(new Job(column, index, numCycles));
}
pool.waitForDone();
}
void KisTiledDataManagerTest::benchmaskQRegion()
{
QVector<QRect> rects;
......
......@@ -63,6 +63,8 @@ private Q_SLOTS:
void stressTestLazyCopying();
void stressTestExtentsColumn();
void benchmaskQRegion();
void benchmaskKisRegion();
};
......
Markdown is supported
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