QCoreApplication: work towards replacing a QRecursiveMutex with a QMutex

At first glance, libraryPathMutex is only recursive because
setLibraryPaths(), addLibraryPath() and removeLibraryPath(), all of
which lock libraryPathMutex, may call libraryPaths(), which does, too.

This is easily fixed by splitting libraryPaths() into public
libraryPaths() and private libraryPathsLocked(), the latter expecting
to be called with the libraryPathMutex already held. And this is what
this patch does.

However, on second glance, the building of the initial app_libpaths
calls a monstrous amount of code, incl. QLibraryInfo, and some of
that code probably re-enters one of the library-path functions.

So while this patch is a step towards making libraryPathMutex
non-recursive, it's probably not the end.

Change-Id: I3ed83272ace6966980cf8e1db877f24c89789da3
Reviewed-by: Ulf Hermann <ulf.hermann@qt.io>
Reviewed-by: Edward Welbourne <edward.welbourne@qt.io>
Reviewed-by: Olivier Goffart (Woboq GmbH) <ogoffart@woboq.com>
This commit is contained in:
Marc Mutz 2019-08-21 22:14:16 +02:00
parent 333b1e9229
commit 08ad96404b
2 changed files with 13 additions and 3 deletions

View File

@ -2693,7 +2693,14 @@ Q_GLOBAL_STATIC(QRecursiveMutex, libraryPathMutex)
QStringList QCoreApplication::libraryPaths()
{
QMutexLocker locker(libraryPathMutex());
return libraryPathsLocked();
}
/*!
\internal
*/
QStringList QCoreApplication::libraryPathsLocked()
{
if (coreappdata()->manual_libpaths)
return *(coreappdata()->manual_libpaths);
@ -2769,7 +2776,7 @@ void QCoreApplication::setLibraryPaths(const QStringList &paths)
// When the application is constructed it should still amend the paths. So we keep the originals
// around, and even create them if they don't exist, yet.
if (!coreappdata()->app_libpaths)
libraryPaths();
libraryPathsLocked();
if (coreappdata()->manual_libpaths)
*(coreappdata()->manual_libpaths) = paths;
@ -2812,7 +2819,7 @@ void QCoreApplication::addLibraryPath(const QString &path)
return;
} else {
// make sure that library paths are initialized
libraryPaths();
libraryPathsLocked();
QStringList *app_libpaths = coreappdata()->app_libpaths.data();
if (app_libpaths->contains(canonicalPath))
return;
@ -2851,7 +2858,7 @@ void QCoreApplication::removeLibraryPath(const QString &path)
return;
} else {
// make sure that library paths is initialized
libraryPaths();
libraryPathsLocked();
QStringList *app_libpaths = coreappdata()->app_libpaths.data();
if (!app_libpaths->contains(canonicalPath))
return;

View File

@ -208,6 +208,9 @@ private:
static bool notifyInternal2(QObject *receiver, QEvent *);
static bool forwardEvent(QObject *receiver, QEvent *event, QEvent *originatingEvent = nullptr);
#endif
#if QT_CONFIG(library)
static QStringList libraryPathsLocked();
#endif
static QCoreApplication *self;