Make ModelManagerInterface::instance() a bit safer

Currently, in d'tor of ModelManagerInterface, when joinAllThreads()
has finished, we were setting the g_instance to nullptr. However,
just after the joinAllThreads() has finished and before setting
the g_instance to nullptr some other thread could still add a
new future and this thread could potentially still assume that
instance() will return valid pointer.

The fix is to make joinAllThreads() and setting the g_instance to
nullptr in an atomic way. We ensure, that when there are still
pending futures, we don't clear the instance, but call joinAllThreads()
again for them.

The implementation is similar to what we do in test_joinAllThreads().

Change-Id: I99dc341228aee710a958a0fbc6c5ec1fbe132e7f
Reviewed-by: Fawzi Mohamed <fawzi.mohamed@qt.io>
This commit is contained in:
Jarek Kobus
2021-06-21 16:40:19 +02:00
parent 2af03178ae
commit 7762cc745c

View File

@@ -142,13 +142,20 @@ ModelManagerInterface::ModelManagerInterface(QObject *parent)
ModelManagerInterface::~ModelManagerInterface()
{
joinAllThreads(true);
Q_ASSERT(g_instance == this);
m_cppQmlTypesUpdater.cancel();
m_cppQmlTypesUpdater.waitForFinished();
QMutexLocker locker(&g_instanceMutex);
Q_ASSERT(g_instance == this);
while (true) {
joinAllThreads(true);
// Keep these 2 mutexes in the same order as inside instanceForFuture()
QMutexLocker instanceLocker(&g_instanceMutex);
QMutexLocker futureLocker(&m_futuresMutex);
if (m_futureSynchronizer.isEmpty()) {
g_instance = nullptr;
return;
}
}
}
static QHash<QString, Dialect> defaultLanguageMapping()
@@ -655,6 +662,7 @@ QList<ModelManagerInterface::ProjectInfo> ModelManagerInterface::allProjectInfos
bool ModelManagerInterface::isIdle() const
{
QMutexLocker futureLocker(&m_futuresMutex);
return m_futureSynchronizer.isEmpty();
}