diff options
author | Ulf Hermann <ulf.hermann@digia.com> | 2014-02-06 13:01:05 +0100 |
---|---|---|
committer | The Qt Project <gerrit-noreply@qt-project.org> | 2014-02-07 10:49:32 +0100 |
commit | 85bab8c8dbe385520753bc77f36b8115be5a046e (patch) | |
tree | ba43e71c66217cecd6d5b1dc4ebbf2785676a510 | |
parent | 0b806c3adda7172673ee76a9d6f4320cf986bbf8 (diff) |
Protect list of client plugins from concurrent access
Previously the list of client plugins in the QQmlDebugServer could be
accessed concurrently from receiveMessage() and addService() either
when in non-blocking mode, or if the client uses service discovery or
sends additional hello messages after the first one.
Change-Id: I946243957184210d40ebca728143714c341b1226
Reviewed-by: Kai Koehne <kai.koehne@digia.com>
-rw-r--r-- | src/qml/debugger/qqmldebugserver.cpp | 76 |
1 files changed, 35 insertions, 41 deletions
diff --git a/src/qml/debugger/qqmldebugserver.cpp b/src/qml/debugger/qqmldebugserver.cpp index 025bbc2113..f1e17df12b 100644 --- a/src/qml/debugger/qqmldebugserver.cpp +++ b/src/qml/debugger/qqmldebugserver.cpp @@ -435,6 +435,7 @@ void QQmlDebugServer::receiveMessage(const QByteArray &message) int op = -1; in >> op; if (op == 0) { + QWriteLocker lock(&d->pluginsLock); int version; in >> version >> d->clientPlugins; @@ -449,25 +450,22 @@ void QQmlDebugServer::receiveMessage(const QByteArray &message) // the plugins below start sending messages. QByteArray helloAnswer; - { - QReadLocker readPluginsLock(&d->pluginsLock); - QQmlDebugStream out(&helloAnswer, QIODevice::WriteOnly); - QStringList pluginNames; - QList<float> pluginVersions; - foreach (QQmlDebugService *service, d->plugins.values()) { - pluginNames << service->name(); - pluginVersions << service->version(); - } - - out << QString(QStringLiteral("QDeclarativeDebugClient")) << 0 << protocolVersion - << pluginNames << pluginVersions << s_dataStreamVersion; + QQmlDebugStream out(&helloAnswer, QIODevice::WriteOnly); + QStringList pluginNames; + QList<float> pluginVersions; + foreach (QQmlDebugService *service, d->plugins.values()) { + pluginNames << service->name(); + pluginVersions << service->version(); } + + out << QString(QStringLiteral("QDeclarativeDebugClient")) << 0 << protocolVersion + << pluginNames << pluginVersions << s_dataStreamVersion; + d->connection->send(QList<QByteArray>() << helloAnswer); QMutexLocker helloLock(&d->helloMutex); d->gotHello = true; - QReadLocker lock(&d->pluginsLock); QHash<QString, QQmlDebugService*>::ConstIterator iter = d->plugins.constBegin(); for (; iter != d->plugins.constEnd(); ++iter) { QQmlDebugService::State newState = QQmlDebugService::Unavailable; @@ -480,12 +478,12 @@ void QQmlDebugServer::receiveMessage(const QByteArray &message) d->helloCondition.wakeAll(); } else if (op == 1) { + QWriteLocker lock(&d->pluginsLock); // Service Discovery QStringList oldClientPlugins = d->clientPlugins; in >> d->clientPlugins; - QReadLocker lock(&d->pluginsLock); QHash<QString, QQmlDebugService*>::ConstIterator iter = d->plugins.constBegin(); for (; iter != d->plugins.constEnd(); ++iter) { const QString pluginName = iter.key(); @@ -533,7 +531,10 @@ void QQmlDebugServerPrivate::_q_changeServiceState(const QString &serviceName, QQmlDebugService *service = 0; { - QReadLocker lock(&pluginsLock); + // Write lock here, because this can be called from receiveMessage which already has a write + // lock. We cannot downgrade it. We also don't want to give up the write lock and later get + // a read lock as that technique has great potential for deadlocks. + QWriteLocker lock(&pluginsLock); service = plugins.value(serviceName); } @@ -604,20 +605,15 @@ bool QQmlDebugServer::addService(QQmlDebugService *service) // to be executed outside of debugger thread Q_ASSERT(QThread::currentThread() != thread()); - { - QWriteLocker lock(&d->pluginsLock); - if (!service || d->plugins.contains(service->name())) - return false; - d->plugins.insert(service->name(), service); - } - { - QReadLocker lock(&d->pluginsLock); - d->advertisePlugins(); - QQmlDebugService::State newState = QQmlDebugService::Unavailable; - if (d->clientPlugins.contains(service->name())) - newState = QQmlDebugService::Enabled; - service->d_func()->state = newState; - } + QWriteLocker lock(&d->pluginsLock); + if (!service || d->plugins.contains(service->name())) + return false; + d->plugins.insert(service->name(), service); + d->advertisePlugins(); + QQmlDebugService::State newState = QQmlDebugService::Unavailable; + if (d->clientPlugins.contains(service->name())) + newState = QQmlDebugService::Enabled; + service->d_func()->state = newState; return true; } @@ -628,21 +624,19 @@ bool QQmlDebugServer::removeService(QQmlDebugService *service) // to be executed outside of debugger thread Q_ASSERT(QThread::currentThread() != thread()); - { - QWriteLocker lock(&d->pluginsLock); - QQmlDebugService::State newState = QQmlDebugService::NotConnected; + QWriteLocker lock(&d->pluginsLock); + QQmlDebugService::State newState = QQmlDebugService::NotConnected; - d->changeServiceStateCalls.ref(); - QMetaObject::invokeMethod(this, "_q_changeServiceState", Qt::QueuedConnection, - Q_ARG(QString, service->name()), - Q_ARG(QQmlDebugService::State, newState)); + d->changeServiceStateCalls.ref(); + QMetaObject::invokeMethod(this, "_q_changeServiceState", Qt::QueuedConnection, + Q_ARG(QString, service->name()), + Q_ARG(QQmlDebugService::State, newState)); - if (!service || !d->plugins.contains(service->name())) - return false; - d->plugins.remove(service->name()); + if (!service || !d->plugins.contains(service->name())) + return false; + d->plugins.remove(service->name()); - d->advertisePlugins(); - } + d->advertisePlugins(); return true; } |