From 1634987caefd36303c048a271ae67ca6b927a001 Mon Sep 17 00:00:00 2001 From: Ulf Hermann Date: Thu, 7 Sep 2017 15:17:15 +0200 Subject: [PATCH] QmlDebug: Don't use QmlDebugConnection unconditionally It might be null. We wrap the retrieval of the data stream version in QmlDebugClient and return the minimum if connection is null. The extra copy of the connection QmlEngine is dropped as QmlDebugClient already has one and we don't want to hit a dangling pointer. Change-Id: Ida8c45d357d46b4942eea99b77065d3c51c7edb9 Reviewed-by: hjk --- src/libs/qmldebug/baseenginedebugclient.cpp | 28 ++++++------ .../qmldebug/declarativeenginedebugclient.cpp | 8 ++-- src/libs/qmldebug/qmldebugclient.cpp | 9 +++- src/libs/qmldebug/qmldebugclient.h | 1 + src/libs/qmldebug/qmltoolsclient.cpp | 8 ++-- src/plugins/debugger/qml/qmlengine.cpp | 43 ++++++++++--------- .../qmlprofiler/qmlprofilertraceclient.cpp | 4 +- 7 files changed, 56 insertions(+), 45 deletions(-) diff --git a/src/libs/qmldebug/baseenginedebugclient.cpp b/src/libs/qmldebug/baseenginedebugclient.cpp index f3e5271e32b..df014e43a00 100644 --- a/src/libs/qmldebug/baseenginedebugclient.cpp +++ b/src/libs/qmldebug/baseenginedebugclient.cpp @@ -178,7 +178,7 @@ void BaseEngineDebugClient::stateChanged(State state) void BaseEngineDebugClient::messageReceived(const QByteArray &data) { - QPacket ds(connection()->currentDataStreamVersion(), data); + QPacket ds(dataStreamVersion(), data); int queryId; QByteArray type; ds >> type >> queryId; @@ -251,7 +251,7 @@ quint32 BaseEngineDebugClient::addWatch(const PropertyReference &property) quint32 id = 0; if (state() == Enabled) { id = getId(); - QPacket ds(connection()->currentDataStreamVersion()); + QPacket ds(dataStreamVersion()); ds << QByteArray("WATCH_PROPERTY") << id << property.m_objectDebugId << property.m_name.toUtf8(); sendMessage(ds.data()); @@ -272,7 +272,7 @@ quint32 BaseEngineDebugClient::addWatch(const ObjectReference &object, quint32 id = 0; if (state() == Enabled) { id = getId(); - QPacket ds(connection()->currentDataStreamVersion()); + QPacket ds(dataStreamVersion()); ds << QByteArray("WATCH_EXPR_OBJECT") << id << object.m_debugId << expr; sendMessage(ds.data()); } @@ -284,7 +284,7 @@ quint32 BaseEngineDebugClient::addWatch(int objectDebugId) quint32 id = 0; if (state() == Enabled) { id = getId(); - QPacket ds(connection()->currentDataStreamVersion()); + QPacket ds(dataStreamVersion()); ds << QByteArray("WATCH_OBJECT") << id << objectDebugId; sendMessage(ds.data()); } @@ -300,7 +300,7 @@ quint32 BaseEngineDebugClient::addWatch(const FileReference &/*file*/) void BaseEngineDebugClient::removeWatch(quint32 id) { if (state() == Enabled) { - QPacket ds(connection()->currentDataStreamVersion()); + QPacket ds(dataStreamVersion()); ds << QByteArray("NO_WATCH") << id; sendMessage(ds.data()); } @@ -311,7 +311,7 @@ quint32 BaseEngineDebugClient::queryAvailableEngines() quint32 id = 0; if (state() == Enabled) { id = getId(); - QPacket ds(connection()->currentDataStreamVersion()); + QPacket ds(dataStreamVersion()); ds << QByteArray("LIST_ENGINES") << id; sendMessage(ds.data()); } @@ -323,7 +323,7 @@ quint32 BaseEngineDebugClient::queryRootContexts(const EngineReference &engine) quint32 id = 0; if (state() == Enabled && engine.m_debugId != -1) { id = getId(); - QPacket ds(connection()->currentDataStreamVersion()); + QPacket ds(dataStreamVersion()); ds << QByteArray("LIST_OBJECTS") << id << engine.m_debugId; sendMessage(ds.data()); } @@ -335,7 +335,7 @@ quint32 BaseEngineDebugClient::queryObject(int objectId) quint32 id = 0; if (state() == Enabled && objectId != -1) { id = getId(); - QPacket ds(connection()->currentDataStreamVersion()); + QPacket ds(dataStreamVersion()); ds << QByteArray("FETCH_OBJECT") << id << objectId << false << true; sendMessage(ds.data()); @@ -348,7 +348,7 @@ quint32 BaseEngineDebugClient::queryObjectRecursive(int objectId) quint32 id = 0; if (state() == Enabled && objectId != -1) { id = getId(); - QPacket ds(connection()->currentDataStreamVersion()); + QPacket ds(dataStreamVersion()); ds << QByteArray("FETCH_OBJECT") << id << objectId << true << true; sendMessage(ds.data()); @@ -363,7 +363,7 @@ quint32 BaseEngineDebugClient::queryExpressionResult(int objectDebugId, quint32 id = 0; if (state() == Enabled && objectDebugId != -1) { id = getId(); - QPacket ds(connection()->currentDataStreamVersion()); + QPacket ds(dataStreamVersion()); ds << QByteArray("EVAL_EXPRESSION") << id << objectDebugId << expr << engineId; sendMessage(ds.data()); @@ -381,7 +381,7 @@ quint32 BaseEngineDebugClient::setBindingForObject( quint32 id = 0; if (state() == Enabled && objectDebugId != -1) { id = getId(); - QPacket ds(connection()->currentDataStreamVersion()); + QPacket ds(dataStreamVersion()); ds << QByteArray("SET_BINDING") << id << objectDebugId << propertyName << bindingExpression << isLiteralValue << source << line; sendMessage(ds.data()); @@ -396,7 +396,7 @@ quint32 BaseEngineDebugClient::resetBindingForObject( quint32 id = 0; if (state() == Enabled && objectDebugId != -1) { id = getId(); - QPacket ds(connection()->currentDataStreamVersion()); + QPacket ds(dataStreamVersion()); ds << QByteArray("RESET_BINDING") << id << objectDebugId << propertyName; sendMessage(ds.data()); } @@ -410,7 +410,7 @@ quint32 BaseEngineDebugClient::setMethodBody( quint32 id = 0; if (state() == Enabled && objectDebugId != -1) { id = getId(); - QPacket ds(connection()->currentDataStreamVersion()); + QPacket ds(dataStreamVersion()); ds << QByteArray("SET_METHOD_BODY") << id << objectDebugId << methodName << methodBody; sendMessage(ds.data()); @@ -424,7 +424,7 @@ quint32 BaseEngineDebugClient::queryObjectsForLocation( quint32 id = 0; if (state() == Enabled) { id = getId(); - QPacket ds(connection()->currentDataStreamVersion()); + QPacket ds(dataStreamVersion()); ds << QByteArray("FETCH_OBJECTS_FOR_LOCATION") << id << fileName << lineNumber << columnNumber << false << true; diff --git a/src/libs/qmldebug/declarativeenginedebugclient.cpp b/src/libs/qmldebug/declarativeenginedebugclient.cpp index e014aa44fac..21a13a960d3 100644 --- a/src/libs/qmldebug/declarativeenginedebugclient.cpp +++ b/src/libs/qmldebug/declarativeenginedebugclient.cpp @@ -46,7 +46,7 @@ quint32 DeclarativeEngineDebugClient::setBindingForObject( quint32 id = 0; if (state() == Enabled && objectDebugId != -1) { id = getId(); - QPacket ds(connection()->currentDataStreamVersion()); + QPacket ds(dataStreamVersion()); ds << QByteArray("SET_BINDING") << objectDebugId << propertyName << bindingExpression << isLiteralValue << source << line; sendMessage(ds.data()); @@ -61,7 +61,7 @@ quint32 DeclarativeEngineDebugClient::resetBindingForObject( quint32 id = 0; if (state() == Enabled && objectDebugId != -1) { id = getId(); - QPacket ds(connection()->currentDataStreamVersion()); + QPacket ds(dataStreamVersion()); ds << QByteArray("RESET_BINDING") << objectDebugId << propertyName; sendMessage(ds.data()); } @@ -75,7 +75,7 @@ quint32 DeclarativeEngineDebugClient::setMethodBody( quint32 id = 0; if (state() == Enabled && objectDebugId != -1) { id = getId(); - QPacket ds(connection()->currentDataStreamVersion()); + QPacket ds(dataStreamVersion()); ds << QByteArray("SET_METHOD_BODY") << objectDebugId << methodName << methodBody; sendMessage(ds.data()); @@ -85,7 +85,7 @@ quint32 DeclarativeEngineDebugClient::setMethodBody( void DeclarativeEngineDebugClient::messageReceived(const QByteArray &data) { - QPacket ds(connection()->currentDataStreamVersion(), data); + QPacket ds(dataStreamVersion(), data); QByteArray type; ds >> type; diff --git a/src/libs/qmldebug/qmldebugclient.cpp b/src/libs/qmldebug/qmldebugclient.cpp index 5935fee5d25..1b27ee2a125 100644 --- a/src/libs/qmldebug/qmldebugclient.cpp +++ b/src/libs/qmldebug/qmldebugclient.cpp @@ -38,6 +38,7 @@ namespace QmlDebug { const int protocolVersion = 1; +const int minimumDataStreamVersion = QDataStream::Qt_4_7; const QString serverId = QLatin1String("QDeclarativeDebugServer"); const QString clientId = QLatin1String("QDeclarativeDebugClient"); @@ -86,7 +87,7 @@ static QString socketErrorToString(QAbstractSocket::SocketError error) QmlDebugConnectionPrivate::QmlDebugConnectionPrivate() : protocol(0), server(0), device(0), gotHello(false), - currentDataStreamVersion(QDataStream::Qt_4_7), + currentDataStreamVersion(minimumDataStreamVersion), maximumDataStreamVersion(QDataStream::Qt_DefaultCompiledVersion) { } @@ -493,6 +494,12 @@ QmlDebugConnection *QmlDebugClient::connection() const return d->connection; } +int QmlDebugClient::dataStreamVersion() const +{ + Q_D(const QmlDebugClient); + return (d->connection ? d->connection->currentDataStreamVersion() : minimumDataStreamVersion); +} + void QmlDebugClient::sendMessage(const QByteArray &message) { Q_D(QmlDebugClient); diff --git a/src/libs/qmldebug/qmldebugclient.h b/src/libs/qmldebug/qmldebugclient.h index 8d2f5712753..b39ec5f3db4 100644 --- a/src/libs/qmldebug/qmldebugclient.h +++ b/src/libs/qmldebug/qmldebugclient.h @@ -95,6 +95,7 @@ public: float serviceVersion() const; State state() const; QmlDebugConnection *connection() const; + int dataStreamVersion() const; virtual void sendMessage(const QByteArray &); virtual void stateChanged(State); diff --git a/src/libs/qmldebug/qmltoolsclient.cpp b/src/libs/qmldebug/qmltoolsclient.cpp index 7ce05ef71aa..e882f0733b3 100644 --- a/src/libs/qmldebug/qmltoolsclient.cpp +++ b/src/libs/qmldebug/qmltoolsclient.cpp @@ -55,7 +55,7 @@ QmlToolsClient::QmlToolsClient(QmlDebugConnection *client) void QmlToolsClient::messageReceived(const QByteArray &message) { - QPacket ds(connection()->currentDataStreamVersion(), message); + QPacket ds(dataStreamVersion(), message); QByteArray type; int requestId; @@ -98,7 +98,7 @@ void QmlToolsClient::setObjectIdList(const QList &objectRoots) foreach (const ObjectReference &object, objectRoots) debugIds << object.debugId(); - QPacket ds(connection()->currentDataStreamVersion()); + QPacket ds(dataStreamVersion()); ds << QByteArray(REQUEST) << m_requestId++ << QByteArray(SELECT) << debugIds; sendMessage(ds.data()); } @@ -108,7 +108,7 @@ void QmlToolsClient::setDesignModeBehavior(bool inDesignMode) if (!m_connection || !m_connection->isConnected()) return; - QPacket ds(connection()->currentDataStreamVersion()); + QPacket ds(dataStreamVersion()); ds << QByteArray(REQUEST) << m_requestId++; if (inDesignMode) ds << QByteArray(ENABLE); @@ -140,7 +140,7 @@ void QmlToolsClient::showAppOnTop(bool showOnTop) if (!m_connection || !m_connection->isConnected()) return; - QPacket ds(connection()->currentDataStreamVersion()); + QPacket ds(dataStreamVersion()); ds << QByteArray(REQUEST) << m_requestId++ << QByteArray(SHOW_APP_ON_TOP) << showOnTop; diff --git a/src/plugins/debugger/qml/qmlengine.cpp b/src/plugins/debugger/qml/qmlengine.cpp index 9ff977fc62e..f5f87fd3054 100644 --- a/src/plugins/debugger/qml/qmlengine.cpp +++ b/src/plugins/debugger/qml/qmlengine.cpp @@ -140,11 +140,10 @@ typedef QHash LookupItems; // id -> (iname, exp) class QmlEnginePrivate : public QmlDebugClient { public: - QmlEnginePrivate(QmlEngine *engine_, QmlDebugConnection *connection_) - : QmlDebugClient("V8Debugger", connection_), + QmlEnginePrivate(QmlEngine *engine_, QmlDebugConnection *connection) + : QmlDebugClient("V8Debugger", connection), engine(engine_), - inspectorAgent(engine, connection_), - connection(connection_) + inspectorAgent(engine, connection) {} void messageReceived(const QByteArray &data); @@ -221,7 +220,6 @@ public: bool contextEvaluate = false; QTimer connectionTimer; - QmlDebug::QmlDebugConnection *connection; QmlDebug::QDebugMessageClient *msgClient = 0; QHash callbackForToken; @@ -250,6 +248,7 @@ QmlEngine::QmlEngine(bool useTerminal) : d(new QmlEnginePrivate(this, new QmlDebugConnection(this))) { setObjectName("QmlEngine"); + QmlDebugConnection *connection = d->connection(); connect(stackHandler(), &StackHandler::stackChanged, this, &QmlEngine::updateCurrentContext); @@ -280,21 +279,21 @@ QmlEngine::QmlEngine(bool useTerminal) connect(&d->connectionTimer, &QTimer::timeout, this, &QmlEngine::checkConnectionState); - connect(d->connection, &QmlDebugConnection::logStateChange, + connect(connection, &QmlDebugConnection::logStateChange, this, &QmlEngine::showConnectionStateMessage); - connect(d->connection, &QmlDebugConnection::logError, this, + connect(connection, &QmlDebugConnection::logError, this, [this](const QString &error) { showMessage("QML Debugger: " + error, LogWarning); }); - connect(d->connection, &QmlDebugConnection::connectionFailed, + connect(connection, &QmlDebugConnection::connectionFailed, this, &QmlEngine::connectionFailed); - connect(d->connection, &QmlDebugConnection::connected, + connect(connection, &QmlDebugConnection::connected, &d->connectionTimer, &QTimer::stop); - connect(d->connection, &QmlDebugConnection::connected, + connect(connection, &QmlDebugConnection::connected, this, &QmlEngine::connectionEstablished); - connect(d->connection, &QmlDebugConnection::disconnected, + connect(connection, &QmlDebugConnection::disconnected, this, &QmlEngine::disconnected); - d->msgClient = new QDebugMessageClient(d->connection); + d->msgClient = new QDebugMessageClient(connection); connect(d->msgClient, &QDebugMessageClient::newState, this, [this](QmlDebugClient::State state) { logServiceStateChange(d->msgClient->name(), d->msgClient->serviceVersion(), state); @@ -404,10 +403,11 @@ void QmlEngine::beginConnection() */ int port = runParameters().qmlServer.port(); - if (!d->connection || d->connection->isConnected()) + QmlDebugConnection *connection = d->connection(); + if (!connection || connection->isConnected()) return; - d->connection->connectToHost(host, port); + connection->connectToHost(host, port); //A timeout to check the connection state d->connectionTimer.start(); @@ -513,8 +513,8 @@ void QmlEngine::closeConnection() if (d->connectionTimer.isActive()) { d->connectionTimer.stop(); } else { - if (d->connection) - d->connection->close(); + if (QmlDebugConnection *connection = d->connection()) + connection->close(); } } @@ -1194,7 +1194,10 @@ void QmlEngine::checkConnectionState() bool QmlEngine::isConnected() const { - return d->connection->isConnected(); + if (QmlDebugConnection *connection = d->connection()) + return connection->isConnected(); + else + return false; } void QmlEngine::showConnectionStateMessage(const QString &message) @@ -1452,7 +1455,7 @@ void QmlEnginePrivate::setBreakpoint(const QString type, const QString target, // } // } if (type == EVENT) { - QPacket rs(connection->currentDataStreamVersion()); + QPacket rs(dataStreamVersion()); rs << target.toUtf8() << enabled; engine->showMessage(QString("%1 %2 %3") .arg(BREAKONSIGNAL, target, QLatin1String(enabled ? "enabled" : "disabled")), LogInput); @@ -1672,7 +1675,7 @@ void QmlEnginePrivate::runDirectCommand(const QString &type, const QByteArray &m engine->showMessage(QString("%1 %2").arg(type, QString::fromLatin1(msg)), LogInput); - QPacket rs(connection->currentDataStreamVersion()); + QPacket rs(dataStreamVersion()); rs << cmd << type.toLatin1() << msg; if (state() == Enabled) @@ -1694,7 +1697,7 @@ void QmlEnginePrivate::memorizeRefs(const QVariant &refs) void QmlEnginePrivate::messageReceived(const QByteArray &data) { - QPacket ds(connection->currentDataStreamVersion(), data); + QPacket ds(dataStreamVersion(), data); QByteArray command; ds >> command; diff --git a/src/plugins/qmlprofiler/qmlprofilertraceclient.cpp b/src/plugins/qmlprofiler/qmlprofilertraceclient.cpp index 76dbf370097..a48123a9a40 100644 --- a/src/plugins/qmlprofiler/qmlprofilertraceclient.cpp +++ b/src/plugins/qmlprofiler/qmlprofilertraceclient.cpp @@ -166,7 +166,7 @@ void QmlProfilerTraceClientPrivate::processCurrentEvent() void QmlProfilerTraceClientPrivate::sendRecordingStatus(int engineId) { - QmlDebug::QPacket stream(q->connection()->currentDataStreamVersion()); + QmlDebug::QPacket stream(q->dataStreamVersion()); stream << recording << engineId; // engineId -1 is OK. It means "all of them" if (recording) { stream << requestedFeatures << flushInterval; @@ -282,7 +282,7 @@ void QmlProfilerTraceClient::stateChanged(State status) void QmlProfilerTraceClient::messageReceived(const QByteArray &data) { - QmlDebug::QPacket stream(connection()->currentDataStreamVersion(), data); + QmlDebug::QPacket stream(dataStreamVersion(), data); stream >> d->currentEvent;