Commit f756798a authored by Igor Kushnir's avatar Igor Kushnir
Browse files

Add Q_ASSERT(m_currentBucket < m_buckets.size())

Several member functions of ItemRepository access elements of m_buckets
at indices up to and including m_currentBucket. Presumably the asserted
condition is an invariant, which must be true at the beginning and at
the end of each public member function of ItemRepository.

Yet the second added assertion in ItemRepository::index() - before the
line `return createIndex(useBucket, indexInBucket);` - fails during each
run of test_itemrepository on my system. The reduced call stack of this
crash is:
qt_assert_x(char const*, char const*, char const*, int) () at /usr/lib/libQt5Core.so.5
KDevelop::ItemRepository<TestItem, TestItemRequest, true, true, 0u, 1048576u>::index(TestItemRequest const&) () at kdevplatform/serialization/itemrepository.h:1349
TestItemRepository::testItemRepository() () at kdevplatform/serialization/tests/test_itemrepository.cpp:145

I added these assertions to debug a rare assertion failure in
ItemRepository::statistics() called by TestItemRepository at the parent
commit. The assertion fails because m_currentBucket is not less than,
but equal to (never greater than) m_buckets.size():
ASSERT failure in QVector<T>::at: "index out of range", file /usr/include/qt/QtCore/qvector.h, line 449
When I run test_itemrepository built in Debug mode 200 times in a Bash
loop, this assertion failure occurred 8 times. So it occurs about 4% of
the time on my system. This assertion failure occasionally occurs on the
KDevelop's CI server too. For example:
https://invent.kde.org/kdevelop/kdevelop/-/jobs/217145
The reduced call stack of this crash is:
QVector<KDevelop::Bucket<TestItem, TestItemRequest, true, 0u>*>::at(int) const (i=873) at /usr/include/qt/QtCore/qvector.h:449
KDevelop::ItemRepository<TestItem, TestItemRequest, true, true, 0u, 1048576u>::bucketForIndex(unsigned short) const (index=873) at kdevplatform/serialization/itemrepository.h:1957
KDevelop::ItemRepository<TestItem, TestItemRequest, true, true, 0u, 1048576u>::statistics() const () at kdevplatform/serialization/itemrepository.h:1639
TestItemRepository::testItemRepository() () at kdevplatform/serialization/tests/test_itemrepository.cpp:208

I have run KDevelop itself built in Debug mode with the added assertions
for about 10 minutes, and no assertion has failed. Perhaps
TestItemRepository's randomized Item size exposes a difficult to
reproduce bug in ItemRepository. Another possible explanation is that
TestItemRepository uses ItemRepository incorrectly, in which case the
test code needs to be fixed.
parent bbf021c6
......@@ -1131,6 +1131,7 @@ public:
if (foundIndexInBucket) {
// 'request' is already present, return the existing index
Q_ASSERT(m_currentBucket < m_buckets.size());
return createIndex(lastBucketWalked, foundIndexInBucket);
}
......@@ -1345,6 +1346,7 @@ public:
if (reOrderFreeSpaceBucketIndex != -1)
updateFreeSpaceOrder(reOrderFreeSpaceBucketIndex);
Q_ASSERT(m_currentBucket < m_buckets.size());
return createIndex(useBucket, indexInBucket);
} else {
//This should never happen when we picked a bucket for re-use
......@@ -1455,6 +1457,8 @@ public:
} else {
putIntoFreeList(bucket, bucketPtr);
}
Q_ASSERT(m_currentBucket < m_buckets.size());
}
using MyDynamicItem = DynamicItem<Item, markForReferenceCounting>;
......@@ -1523,6 +1527,7 @@ public:
bucketPtr = m_buckets.at(bucket);
}
unsigned short indexInBucket = index & 0xffff;
Q_ASSERT(m_currentBucket < m_buckets.size());
return bucketPtr->itemFromIndex(indexInBucket);
}
......@@ -1589,6 +1594,7 @@ public:
Statistics statistics() const
{
Q_ASSERT(m_currentBucket < m_buckets.size());
Statistics ret;
uint loadedBuckets = 0;
for (auto* bucket : m_buckets) {
......
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