Could this be a possible bug for Qt’s SQL database?

[nyaruko]

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;
}

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;
}

To copy to clipboard, switch view to plain text mode 

However, I have met one crash (not every time) right after I call the:

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

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

To copy to clipboard, switch view to plain text mode 

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)

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

To copy to clipboard, switch view to plain text mode 

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...1shared_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]

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");

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

To copy to clipboard, switch view to plain text mode 

[nyaruko]

Yes, I did merge it. But there's still crash. And the crash still comes out from the = operator.

[wysota]

How about putting a muted around calls that add or remove databases?

[pitonyak]

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	}

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

To copy to clipboard, switch view to plain text mode 

the assigment operator is as follows:

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

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

To copy to clipboard, switch view to plain text mode 

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	}

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	}

To copy to clipboard, switch view to plain text mode 

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

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

[wysota]

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]

Btw, might I suggest using QReadLocker for your main lock?
Currently you have at least one return without a matching unlock()

Cheers,
_