PDA

View Full Version : Could this be a possible bug for Qt’s SQL database?



nyaruko
9th November 2014, 19:50
I have the following code which is called by many threads using a thread pool:


QByteArray PureImageCache::GetImageFromCache(MapType::Types type, Point pos, int zoom)

{

lock.lockForRead();

QByteArray ar;

if(gtilecache.isEmpty()|gtilecache.isNull())

return ar;

QString dir=gtilecache;
Mcounter.lock();
qlonglong id=++ConnCounter;
Mcounter.unlock();

QString db=dir+"Data.qmdb";
{
QSqlDatabase cn;

cn = QSqlDatabase::addDatabase("QSQLITE",QString::number(id));

cn.setDatabaseName(db);
cn.setConnectOptions("QSQLITE_ENABLE_SHARED_CACHE");
if(cn.open())
{
QSqlQuery query(cn);
query.exec(QString("SELECT Tile FROM TilesData WHERE id = (SELECT id FROM Tiles WHERE X=%1 AND Y=%2 AND Zoom=%3 AND Type=%4)").arg(pos.X()).arg(pos.Y()).arg(zoom).arg((int) type));
query.next();
if(query.isValid())
{
ar=query.value(0).toByteArray();
//qDebug()<<"Got";
}else{
//qDebug()<<"Miss";
}
cn.close();
}
}
QSqlDatabase::removeDatabase(QString::number(id));
lock.unlock();
return ar;
}
However, I have met one crash (not every time) right after I call the:


cn = QSqlDatabase::addDatabase(“QSQLITE”,QS tring::number(id));

function. And the call stack says the error is:


Debug Assertion Failed!
File:f:\dd\vctools\crt_bld\self_x86\crt\src\dbgdel .cpp
Line: 52
Expression:_BLOCK_TYPE_IS_VALID(pHead->nBlockUse)

The cause is within the = operator, it call an atomic assign where inside that atomic assign, there's a delete function cause the crash.
I noticed that there's a shared_null pointer inside the default constructor of QSqlDatabase.
Here is the thing:
The default constructor of QSqlDatabase used a shared_null.
Note that the shared_null is an object indicating an invalid database, not a plain null pointer.

Here is the code: http://code.woboq.org/qt5/qtbase/src/sql/kernel/qsqldatabase.cpp.html#_ZN19QSqlDatabasePrivate11sh ared_nullEv
Could it be that the delete is called on to delete the shared_null which is essentially some static variable and caused the crash?
How could I avoid this and continue using QSqlDataBase safely in multi-thread senario?

ChrisW67
9th November 2014, 21:00
What happens when you merge lines 20 and 22 so that it looks like every example in the docs and you never default construct an invalid QSqlDatabase?


QSqlDatabase cn = QSqlDatbase::addDatabase("QSQLITE");

nyaruko
9th November 2014, 21:18
Yes, I did merge it. But there's still crash. And the crash still comes out from the = operator.

wysota
12th November 2014, 09:48
How about putting a muted around calls that add or remove databases?

pitonyak
13th November 2014, 21:53
Off hand, it looks like there is a test to not attempt a deletion in the destructor of the



181 QSqlDatabasePrivate::~QSqlDatabasePrivate()
182 {
183 if (driver != shared_null()->driver)
184 delete driver;
185 }

the assigment operator is as follows:


710 QSqlDatabase &QSqlDatabase::operator=(const QSqlDatabase &other)
711 {
712 qAtomicAssign(d, other.d);
713 return *this;
714 }

The type d is on the private database type, so, even though qAtomicAssign will indeed delete the other.d, that destructor will not cause the shared NULL to be deleted. The value for d is then immediately assigned a new value.


template <typename T>
232 inline void qAtomicAssign(T *&d, T *x)
233 {
234 if (d == x)
235 return;
236 x->ref.ref();
237 if (!d->ref.deref())
238 delete d;
239 d = x;
240 }

I am not saying that you are wrong, but, I am saying that


The code looks OK from my quick peek.
I don't see any problems in your code either

:(

wysota
13th November 2014, 22:47
This is a code of a single object but Qt has to store a list of registered databases somewhere. I think you might just add the mutex and see if it changes anything.

anda_skoa
14th November 2014, 09:17
Btw, might I suggest using QReadLocker for your main lock?
Currently you have at least one return without a matching unlock()

Cheers,
_