Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
QmlProfiler: Avoid race conditions in QQmlTypeLoader
We have to make sure all profiler calls to one adapter are done from the
same thread.

It turns out that all the calls from QQmlTypeLoader are done from the
type loader thread. By using a separate adapter for that, we avoid any
extra locking.

Task-number: QTBUG-62987
Change-Id: Ice400b1c3b7bd920d855ceb1ba0d46417f50348d
(cherry picked from commit 4578a92)
Reviewed-by: Simon Hausmann <simon.hausmann@qt.io>
  • Loading branch information
Ulf Hermann authored and gladhorn committed Sep 7, 2017
1 parent a8cff0a commit cf3b1bb
Show file tree
Hide file tree
Showing 5 changed files with 62 additions and 15 deletions.
29 changes: 21 additions & 8 deletions src/plugins/qmltooling/qmldbg_profiler/qqmlprofileradapter.cpp
Expand Up @@ -37,19 +37,32 @@
QT_BEGIN_NAMESPACE

QQmlProfilerAdapter::QQmlProfilerAdapter(QQmlProfilerService *service, QQmlEnginePrivate *engine) :
QQmlAbstractProfilerAdapter(service), next(0)
QQmlAbstractProfilerAdapter(service)
{
engine->enableProfiler();
connect(this, SIGNAL(profilingEnabled(quint64)), engine->profiler, SLOT(startProfiling(quint64)));
init(engine->profiler);
}

QQmlProfilerAdapter::QQmlProfilerAdapter(QQmlProfilerService *service, QQmlTypeLoader *loader) :
QQmlAbstractProfilerAdapter(service)
{
loader->enableProfiler();
init(loader->profiler());
}

void QQmlProfilerAdapter::init(QQmlProfiler *profiler)
{
next = 0;
connect(this, SIGNAL(profilingEnabled(quint64)), profiler, SLOT(startProfiling(quint64)));
connect(this, SIGNAL(profilingEnabledWhileWaiting(quint64)),
engine->profiler, SLOT(startProfiling(quint64)), Qt::DirectConnection);
connect(this, SIGNAL(profilingDisabled()), engine->profiler, SLOT(stopProfiling()));
profiler, SLOT(startProfiling(quint64)), Qt::DirectConnection);
connect(this, SIGNAL(profilingDisabled()), profiler, SLOT(stopProfiling()));
connect(this, SIGNAL(profilingDisabledWhileWaiting()),
engine->profiler, SLOT(stopProfiling()), Qt::DirectConnection);
connect(this, SIGNAL(dataRequested()), engine->profiler, SLOT(reportData()));
profiler, SLOT(stopProfiling()), Qt::DirectConnection);
connect(this, SIGNAL(dataRequested()), profiler, SLOT(reportData()));
connect(this, SIGNAL(referenceTimeKnown(QElapsedTimer)),
engine->profiler, SLOT(setTimer(QElapsedTimer)));
connect(engine->profiler, SIGNAL(dataReady(QVector<QQmlProfilerData>)),
profiler, SLOT(setTimer(QElapsedTimer)));
connect(profiler, SIGNAL(dataReady(QVector<QQmlProfilerData>)),
this, SLOT(receiveData(QVector<QQmlProfilerData>)));
}

Expand Down
2 changes: 2 additions & 0 deletions src/plugins/qmltooling/qmldbg_profiler/qqmlprofileradapter.h
Expand Up @@ -54,12 +54,14 @@ class QQmlProfilerAdapter : public QQmlAbstractProfilerAdapter {
Q_OBJECT
public:
QQmlProfilerAdapter(QQmlProfilerService *service, QQmlEnginePrivate *engine);
QQmlProfilerAdapter(QQmlProfilerService *service, QQmlTypeLoader *loader);
qint64 sendMessages(qint64 until, QList<QByteArray> &messages);

public slots:
void receiveData(const QVector<QQmlProfilerData> &new_data);

private:
void init(QQmlProfiler *profiler);
QVector<QQmlProfilerData> data;
int next;
};
Expand Down
Expand Up @@ -98,9 +98,14 @@ void QQmlProfilerServiceImpl::engineAboutToBeAdded(QQmlEngine *engine)
"QML profilers have to be added from the engine thread");

QMutexLocker lock(&m_configMutex);
QQmlProfilerAdapter *qmlAdapter = new QQmlProfilerAdapter(this, QQmlEnginePrivate::get(engine));
QV4ProfilerAdapter *v4Adapter = new QV4ProfilerAdapter(this, QV8Engine::getV4(engine->handle()));
QQmlEnginePrivate *enginePrivate = QQmlEnginePrivate::get(engine);
QQmlProfilerAdapter *qmlAdapter = new QQmlProfilerAdapter(this, enginePrivate);
QQmlProfilerAdapter *compileAdapter
= new QQmlProfilerAdapter(this, &(enginePrivate->typeLoader));
QV4ProfilerAdapter *v4Adapter
= new QV4ProfilerAdapter(this, QV8Engine::getV4(engine->handle()));
addEngineProfiler(qmlAdapter, engine);
addEngineProfiler(compileAdapter, engine);
addEngineProfiler(v4Adapter, engine);
QQmlConfigurableDebugService<QQmlProfilerService>::engineAboutToBeAdded(engine);
}
Expand Down
20 changes: 16 additions & 4 deletions src/qml/qml/qqmltypeloader.cpp
Expand Up @@ -647,8 +647,7 @@ void QQmlDataBlob::notifyComplete(QQmlDataBlob *blob)
{
Q_ASSERT(m_waitingFor.contains(blob));
Q_ASSERT(blob->status() == Error || blob->status() == Complete);
QQmlCompilingProfiler prof(QQmlEnginePrivate::get(typeLoader()->engine())->profiler,
blob->url());
QQmlCompilingProfiler prof(typeLoader()->profiler(), blob->url());

m_inCallback = true;

Expand Down Expand Up @@ -901,6 +900,12 @@ void QQmlTypeLoader::invalidate()
m_networkReplies.clear();
}

void QQmlTypeLoader::enableProfiler()
{
Q_ASSERT(!m_profiler);
m_profiler = new QQmlProfiler;
}

void QQmlTypeLoader::lock()
{
m_thread->lock();
Expand Down Expand Up @@ -1216,7 +1221,7 @@ void QQmlTypeLoader::setData(QQmlDataBlob *blob, QQmlFile *file)
void QQmlTypeLoader::setData(QQmlDataBlob *blob, const QQmlDataBlob::Data &d)
{
QML_MEMORY_SCOPE_URL(blob->url());
QQmlCompilingProfiler prof(QQmlEnginePrivate::get(engine())->profiler, blob->url());
QQmlCompilingProfiler prof(profiler(), blob->url());

blob->m_inCallback = true;

Expand All @@ -1236,7 +1241,7 @@ void QQmlTypeLoader::setData(QQmlDataBlob *blob, const QQmlDataBlob::Data &d)
void QQmlTypeLoader::setCachedUnit(QQmlDataBlob *blob, const QQmlPrivate::CachedQmlUnit *unit)
{
QML_MEMORY_SCOPE_URL(blob->url());
QQmlCompilingProfiler prof(QQmlEnginePrivate::get(engine())->profiler, blob->url());
QQmlCompilingProfiler prof(profiler(), blob->url());

blob->m_inCallback = true;

Expand Down Expand Up @@ -1594,6 +1599,9 @@ Constructs a new type loader that uses the given \a engine.
*/
QQmlTypeLoader::QQmlTypeLoader(QQmlEngine *engine)
: m_engine(engine), m_thread(new QQmlTypeLoaderThread(this)),
#ifndef QT_NO_QML_DEBUGGER
m_profiler(0),
#endif
m_typeCacheTrimThreshold(TYPELOADER_MINIMUM_TRIM_THRESHOLD)
{
}
Expand All @@ -1610,6 +1618,10 @@ QQmlTypeLoader::~QQmlTypeLoader()
clearCache();

invalidate();

#ifndef QT_NO_QML_DEBUGGER
delete m_profiler;
#endif
}

QQmlImportDatabase *QQmlTypeLoader::importDatabase()
Expand Down
17 changes: 16 additions & 1 deletion src/qml/qml/qqmltypeloader_p.h
Expand Up @@ -74,6 +74,7 @@ class QQmlComponentPrivate;
class QQmlTypeData;
class QQmlTypeLoader;
class QQmlExtensionInterface;
class QQmlProfiler;

namespace QmlIR {
struct Document;
Expand Down Expand Up @@ -211,7 +212,7 @@ class Q_QML_PRIVATE_EXPORT QQmlDataBlob : public QQmlRefCount

class QQmlTypeLoaderThread;

class Q_AUTOTEST_EXPORT QQmlTypeLoader
class Q_QML_PRIVATE_EXPORT QQmlTypeLoader
{
Q_DECLARE_TR_FUNCTIONS(QQmlTypeLoader)
public:
Expand Down Expand Up @@ -311,6 +312,15 @@ class Q_AUTOTEST_EXPORT QQmlTypeLoader
void initializeEngine(QQmlExtensionInterface *, const char *);
void invalidate();

#ifdef QT_NO_QML_DEBUGGER
QQmlProfiler *profiler() const { return 0; }
void enableProfiler() {}
#else
QQmlProfiler *profiler() const { return m_profiler; }
void enableProfiler();
#endif // QT_NO_QML_DEBUGGER


private:
friend class QQmlDataBlob;
friend class QQmlTypeLoaderThread;
Expand Down Expand Up @@ -356,6 +366,11 @@ class Q_AUTOTEST_EXPORT QQmlTypeLoader

QQmlEngine *m_engine;
QQmlTypeLoaderThread *m_thread;

#ifndef QT_NO_QML_DEBUGGER
QQmlProfiler *m_profiler;
#endif

NetworkReplies m_networkReplies;
TypeCache m_typeCache;
int m_typeCacheTrimThreshold;
Expand Down

0 comments on commit cf3b1bb

Please sign in to comment.