Skip to content

Commit

Permalink
Fix race condition in QFactoryLoader: lock the mutex we already have
Browse files Browse the repository at this point in the history
The process of loading a plugin is examplified by the qLoadPlugin
function (though not all users of QFactoryLoader use this function, they
all do something similar):

    const int index = loader->indexOf(key);
    if (index != -1) {
        QObject *factoryObject = loader->instance(index);
        if (FactoryInterface *factory = qobject_cast<FactoryInterface *>(factoryObject))
            if (PluginInterface *result = factory->create(key, std::forward<Args>(args)...))
                return result;
    }

QFactoryLoader::indexOf already locked the mutex, but not
QFactoryLoader::instance. This commit fixes that.

Note that calling the virtual create() in the plugin's factory is not
protected by the mutex. Each plugin's factory must be thread-safe and
also create an object that works on any thread too. It's also the
responsibility of the caller of qLoadPlugin to ensure that it's called
thread-safely.

Task-number: QTBUG-42855
Change-Id: I63e21df51c7448bc8b5ffffd148ebee33d4c47de
Reviewed-by: Olivier Goffart (Woboq GmbH) <[email protected]>
Reviewed-by: Marc Mutz <[email protected]>
  • Loading branch information
thiagomacieira committed Dec 17, 2016
1 parent 8325808 commit 7ca66b1
Showing 1 changed file with 2 additions and 0 deletions.
2 changes: 2 additions & 0 deletions src/corelib/plugin/qfactoryloader.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -282,6 +282,7 @@ QObject *QFactoryLoader::instance(int index) const
return 0;

#ifndef QT_NO_LIBRARY
QMutexLocker lock(&d->mutex);
if (index < d->libraryList.size()) {
QLibraryPrivate *library = d->libraryList.at(index);
if (library->instance || library->loadPlugin()) {
Expand All @@ -297,6 +298,7 @@ QObject *QFactoryLoader::instance(int index) const
return 0;
}
index -= d->libraryList.size();
lock.unlock();
#endif

QVector<QStaticPlugin> staticPlugins = QPluginLoader::staticPlugins();
Expand Down

0 comments on commit 7ca66b1

Please sign in to comment.