From 92b4c1abcfc64855cd73733f136a86435e562beb Mon Sep 17 00:00:00 2001 From: Kai Koehne Date: Mon, 21 Nov 2011 16:03:53 +0100 Subject: [PATCH] Debugger: Make registration of services explicit Services now have to call registerService() themselves in the constructor. This fixes a race condition where the empty implementation of messageReceived() was called instead of the one in the subclass because the object wasn't fully constructed yet. Change-Id: I590ec8b76e906bdb6b5cdcb18680938edde283ee Reviewed-by: Aurindam Jana --- .../debugger/qdeclarativedebugservice.cpp | 27 +++++++++---------- .../debugger/qdeclarativedebugservice_p.h | 3 ++ .../debugger/qdeclarativedebugtrace.cpp | 3 +- .../debugger/qdeclarativeenginedebugservice.cpp | 2 + .../debugger/qdeclarativeinspectorservice.cpp | 1 + src/declarative/debugger/qv8debugservice.cpp | 2 +- src/declarative/debugger/qv8profilerservice.cpp | 3 +- .../tst_qdeclarativedebugservice.cpp | 5 +-- .../auto/declarative/debugger/shared/debugutil.cpp | 1 + 9 files changed, 27 insertions(+), 20 deletions(-) diff --git a/src/declarative/debugger/qdeclarativedebugservice.cpp b/src/declarative/debugger/qdeclarativedebugservice.cpp index be60ea4..0a997aa 100644 --- a/src/declarative/debugger/qdeclarativedebugservice.cpp +++ b/src/declarative/debugger/qdeclarativedebugservice.cpp @@ -61,14 +61,9 @@ QDeclarativeDebugService::QDeclarativeDebugService(const QString &name, QObject d->server = QDeclarativeDebugServer::instance(); d->status = QDeclarativeDebugService::NotConnected; - if (!d->server) - return; - - if (d->server->serviceNames().contains(name)) { - qWarning() << "QDeclarativeDebugService: Conflicting plugin name" << name; + if (d->server->serviceNames().contains(d->name)) { + qWarning() << "QDeclarativeDebugService: Conflicting plugin name" << d->name; d->server = 0; - } else { - d->server->addService(this); } } @@ -80,16 +75,20 @@ QDeclarativeDebugService::QDeclarativeDebugService(QDeclarativeDebugServicePriva d->name = name; d->server = QDeclarativeDebugServer::instance(); d->status = QDeclarativeDebugService::NotConnected; +} +/** + Registers the service. This should be called in the constructor of the inherited class. From + then on the service might get asynchronous calls to messageReceived(). + */ +QDeclarativeDebugService::Status QDeclarativeDebugService::registerService() +{ + Q_D(QDeclarativeDebugService); if (!d->server) - return; + return NotConnected; - if (d->server->serviceNames().contains(name)) { - qWarning() << "QDeclarativeDebugService: Conflicting plugin name" << name; - d->server = 0; - } else { - d->server->addService(this); - } + d->server->addService(this); + return status(); } QDeclarativeDebugService::~QDeclarativeDebugService() diff --git a/src/declarative/debugger/qdeclarativedebugservice_p.h b/src/declarative/debugger/qdeclarativedebugservice_p.h index 05580ee..3bcf310 100644 --- a/src/declarative/debugger/qdeclarativedebugservice_p.h +++ b/src/declarative/debugger/qdeclarativedebugservice_p.h @@ -92,6 +92,9 @@ public: protected: QDeclarativeDebugService(QDeclarativeDebugServicePrivate &dd, const QString &, QObject *parent = 0); + + Status registerService(); + virtual void statusChanged(Status); virtual void messageReceived(const QByteArray &); diff --git a/src/declarative/debugger/qdeclarativedebugtrace.cpp b/src/declarative/debugger/qdeclarativedebugtrace.cpp index b497f6a..661dd55 100644 --- a/src/declarative/debugger/qdeclarativedebugtrace.cpp +++ b/src/declarative/debugger/qdeclarativedebugtrace.cpp @@ -78,7 +78,8 @@ QDeclarativeDebugTrace::QDeclarativeDebugTrace() m_enabled(false), m_messageReceived(false) { m_timer.start(); - if (status() == Enabled) { + + if (registerService() == Enabled) { // wait for first message indicating whether to trace or not while (!m_messageReceived) waitForMessage(); diff --git a/src/declarative/debugger/qdeclarativeenginedebugservice.cpp b/src/declarative/debugger/qdeclarativeenginedebugservice.cpp index ac188b0..501e600 100644 --- a/src/declarative/debugger/qdeclarativeenginedebugservice.cpp +++ b/src/declarative/debugger/qdeclarativeenginedebugservice.cpp @@ -72,6 +72,8 @@ QDeclarativeEngineDebugService::QDeclarativeEngineDebugService(QObject *parent) { QObject::connect(m_watch, SIGNAL(propertyChanged(int,int,QMetaProperty,QVariant)), this, SLOT(propertyChanged(int,int,QMetaProperty,QVariant))); + + registerService(); } QDataStream &operator<<(QDataStream &ds, diff --git a/src/declarative/debugger/qdeclarativeinspectorservice.cpp b/src/declarative/debugger/qdeclarativeinspectorservice.cpp index 90ab6ad..33c9429 100644 --- a/src/declarative/debugger/qdeclarativeinspectorservice.cpp +++ b/src/declarative/debugger/qdeclarativeinspectorservice.cpp @@ -59,6 +59,7 @@ QDeclarativeInspectorService::QDeclarativeInspectorService() : QDeclarativeDebugService(QLatin1String("QDeclarativeObserverMode")) , m_currentInspectorPlugin(0) { + registerService(); } QDeclarativeInspectorService *QDeclarativeInspectorService::instance() diff --git a/src/declarative/debugger/qv8debugservice.cpp b/src/declarative/debugger/qv8debugservice.cpp index f708f59..40af52c 100644 --- a/src/declarative/debugger/qv8debugservice.cpp +++ b/src/declarative/debugger/qv8debugservice.cpp @@ -136,7 +136,7 @@ QV8DebugService::QV8DebugService(QObject *parent) // profiler in Qt Creator. v8::Debug::GetDebugContext(); - if (status() == Enabled) { + if (registerService() == Enabled) { // ,block mode, client attached while (!d->initialized) { waitForMessage(); diff --git a/src/declarative/debugger/qv8profilerservice.cpp b/src/declarative/debugger/qv8profilerservice.cpp index 807fe83..279992a 100644 --- a/src/declarative/debugger/qv8profilerservice.cpp +++ b/src/declarative/debugger/qv8profilerservice.cpp @@ -104,7 +104,8 @@ QV8ProfilerService::QV8ProfilerService(QObject *parent) : QDeclarativeDebugService(*(new QV8ProfilerServicePrivate()), QLatin1String("V8Profiler"), parent) { Q_D(QV8ProfilerService); - if (status() == Enabled) { + + if (registerService() == Enabled) { // ,block mode, client attached while (!d->initialized) waitForMessage(); diff --git a/tests/auto/declarative/debugger/qdeclarativedebugservice/tst_qdeclarativedebugservice.cpp b/tests/auto/declarative/debugger/qdeclarativedebugservice/tst_qdeclarativedebugservice.cpp index 6063c1f..21aeca9 100644 --- a/tests/auto/declarative/debugger/qdeclarativedebugservice/tst_qdeclarativedebugservice.cpp +++ b/tests/auto/declarative/debugger/qdeclarativedebugservice/tst_qdeclarativedebugservice.cpp @@ -118,8 +118,7 @@ void tst_QDeclarativeDebugService::status() QTRY_COMPARE(service.status(), QDeclarativeDebugService::Unavailable); QTest::ignoreMessage(QtWarningMsg, "QDeclarativeDebugService: Conflicting plugin name \"tst_QDeclarativeDebugService::status()\" "); - - QDeclarativeDebugService duplicate("tst_QDeclarativeDebugService::status()"); + QDeclarativeDebugTestService duplicate("tst_QDeclarativeDebugService::status()"); QCOMPARE(duplicate.status(), QDeclarativeDebugService::NotConnected); } @@ -138,7 +137,7 @@ void tst_QDeclarativeDebugService::sendMessage() QCOMPARE(resp, msg); QTest::ignoreMessage(QtWarningMsg, "QDeclarativeDebugService: Conflicting plugin name \"tst_QDeclarativeDebugService::sendMessage()\" "); - QDeclarativeDebugService duplicate("tst_QDeclarativeDebugService::sendMessage()"); + QDeclarativeDebugTestService duplicate("tst_QDeclarativeDebugService::sendMessage()"); duplicate.sendMessage("msg"); } diff --git a/tests/auto/declarative/debugger/shared/debugutil.cpp b/tests/auto/declarative/debugger/shared/debugutil.cpp index 7af3d0f..b0ba5f2 100644 --- a/tests/auto/declarative/debugger/shared/debugutil.cpp +++ b/tests/auto/declarative/debugger/shared/debugutil.cpp @@ -62,6 +62,7 @@ bool QDeclarativeDebugTest::waitForSignal(QObject *receiver, const char *member, QDeclarativeDebugTestService::QDeclarativeDebugTestService(const QString &s, QObject *parent) : QDeclarativeDebugService(s, parent) { + registerService(); } void QDeclarativeDebugTestService::messageReceived(const QByteArray &ba) -- 1.7.2.5