From 7762cc745ce0ec65ade15d72a38b87b6ff0c2867 Mon Sep 17 00:00:00 2001 From: Jarek Kobus Date: Mon, 21 Jun 2021 16:40:19 +0200 Subject: [PATCH] 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 --- src/libs/qmljs/qmljsmodelmanagerinterface.cpp | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/src/libs/qmljs/qmljsmodelmanagerinterface.cpp b/src/libs/qmljs/qmljsmodelmanagerinterface.cpp index 8eae3c36254..f5001140165 100644 --- a/src/libs/qmljs/qmljsmodelmanagerinterface.cpp +++ b/src/libs/qmljs/qmljsmodelmanagerinterface.cpp @@ -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); - g_instance = nullptr; + 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 defaultLanguageMapping() @@ -655,6 +662,7 @@ QList ModelManagerInterface::allProjectInfos bool ModelManagerInterface::isIdle() const { + QMutexLocker futureLocker(&m_futuresMutex); return m_futureSynchronizer.isEmpty(); }