Fix restart/nullptr crash

The nominal case for a QObject pointer is to emit a change when the
value changes, i.e. changing the pointer or setting to nullptr.  No
signal is emitted when the value is set to the same value again.

However, if the server is restarted, it doesn't know what previous
values were sent and will send the current values.  An edge-case, where
the pointer was set to nullptr and nullptr is received again, wasn't
properly handled in QtRO before this fix.

Change-Id: I11443d69476c5b6fecda11e38919542de8a24eb1
Fixes: QTBUG-74221
Reviewed-by: Michael Brasser <michael.brasser@live.com>

12 files changed, 518 insertions, 3 deletions

diff --git a/src/remoteobjects/qremoteobjectnode.cpp b/src/remoteobjects/qremoteobjectnode.cpp
index edbc4f5..11dfb43 100644
--- a/src/remoteobjects/qremoteobjectnode.cpp
+++ b/src/remoteobjects/qremoteobjectnode.cpp
@@ -1757,9 +1757,10 @@ QVariant QRemoteObjectNodePrivate::handlePointerToQObjectProperty(QConnectedRepl
     Q_ASSERT(property.canConvert<QRO_>());
     QRO_ childInfo = property.value<QRO_>();
     qROPrivDebug() << "QRO_:" << childInfo.name << replicas.contains(childInfo.name) << replicas.keys();
-    if (replicas.contains(childInfo.name) && childInfo.isNull) {
+    if (childInfo.isNull) {
         // Either the source has changed the pointer and we need to update it, or the source pointer is a nullptr
-        replicas.remove(childInfo.name);
+        if (replicas.contains(childInfo.name))
+            replicas.remove(childInfo.name);
         if (childInfo.type == ObjectType::CLASS)
             retval = QVariant::fromValue<QRemoteObjectDynamicReplica*>(nullptr);
         else
diff --git a/tests/auto/auto.pro b/tests/auto/auto.pro
index 6da145d..28ee54f 100644
--- a/tests/auto/auto.pro
+++ b/tests/auto/auto.pro
@@ -24,4 +24,4 @@ SUBDIRS += \
 contains(QT_CONFIG, ssl): SUBDIRS += external_IODevice
 
 qtHaveModule(qml): SUBDIRS += qml
-qtConfig(process): SUBDIRS += integration_multiprocess integration_external
+qtConfig(process): SUBDIRS += integration_multiprocess integration_external restart
diff --git a/tests/auto/restart/client/client.pro b/tests/auto/restart/client/client.pro
new file mode 100644
index 0000000..ce435c2
--- /dev/null
+++ b/tests/auto/restart/client/client.pro
@@ -0,0 +1,16 @@
+TEMPLATE = app
+QT       += remoteobjects core testlib
+QT       -= gui
+
+TARGET = client
+DESTDIR = ./
+CONFIG   += c++11
+CONFIG   -= app_bundle
+
+REPC_REPLICA = ../subclass.rep
+
+SOURCES += main.cpp \
+
+HEADERS += \
+
+INCLUDEPATH += $$PWD
diff --git a/tests/auto/restart/client/main.cpp b/tests/auto/restart/client/main.cpp
new file mode 100644
index 0000000..aa2cdd9
--- /dev/null
+++ b/tests/auto/restart/client/main.cpp
@@ -0,0 +1,106 @@
+/****************************************************************************
+**
+** Copyright (C) 2019 Ford Motor Company
+** Contact: https://www.qt.io/licensing/
+**
+** This file is part of the QtRemoteObjects module of the Qt Toolkit.
+**
+** $QT_BEGIN_LICENSE:GPL-EXCEPT$
+** Commercial License Usage
+** Licensees holding valid commercial Qt licenses may use this file in
+** accordance with the commercial license agreement provided with the
+** Software or, alternatively, in accordance with the terms contained in
+** a written agreement between you and The Qt Company. For licensing terms
+** and conditions see https://www.qt.io/terms-conditions. For further
+** information use the contact form at https://www.qt.io/contact-us.
+**
+** GNU General Public License Usage
+** Alternatively, this file may be used under the terms of the GNU
+** General Public License version 3 as published by the Free Software
+** Foundation with exceptions as appearing in the file LICENSE.GPL3-EXCEPT
+** included in the packaging of this file. Please review the following
+** information to ensure the GNU General Public License requirements will
+** be met: https://www.gnu.org/licenses/gpl-3.0.html.
+**
+** $QT_END_LICENSE$
+**
+****************************************************************************/
+
+#include "rep_subclass_replica.h"
+
+#include <QCoreApplication>
+#include <QtRemoteObjects/qremoteobjectnode.h>
+#include <QtTest/QtTest>
+
+class tst_Client_Process : public QObject
+{
+    Q_OBJECT
+
+private Q_SLOTS:
+    void initTestCase()
+    {
+        QLoggingCategory::setFilterRules("qt.remoteobjects.warning=true");
+        m_repNode.reset(new QRemoteObjectNode);
+        m_repNode->connectToNode(QUrl(QStringLiteral("tcp://127.0.0.1:65217")));
+        m_rep.reset(m_repNode->acquire<ParentClassReplica>());
+        connect(m_rep.data(), &QRemoteObjectReplica::stateChanged, [this](QRemoteObjectReplica::State state, QRemoteObjectReplica::State previousState) {
+            qDebug() << "**** stateChanged" << state << previousState;
+            if (state == QRemoteObjectReplica::Suspect) {
+                qWarning() << "Replica suspect";
+                this->serverRestarted = true;
+            } else if (state == QRemoteObjectReplica::Valid) {
+                if (this->serverRestarted) {
+                    qWarning() << "Replica valid again";
+                    auto reply = m_rep->start();
+                }
+            }
+        });
+        QVERIFY(m_rep->waitForSource());
+    }
+
+    void testRun()
+    {
+        const auto objectMode = qEnvironmentVariable("ObjectMode");
+
+        qWarning() << "From client";
+        const MyPOD initialValue(42, 3.14f, QStringLiteral("SubClass"));
+        if (objectMode == QLatin1Literal("ObjectPointer")) {
+            QSignalSpy tracksSpy(m_rep->tracks(), &QAbstractItemModelReplica::initialized);
+            QVERIFY(m_rep->subClass() != nullptr);
+            QCOMPARE(m_rep->subClass()->myPOD(), initialValue);
+            QVERIFY(m_rep->tracks() != nullptr);
+            QVERIFY(tracksSpy.count() || tracksSpy.wait());
+        } else {
+            QVERIFY(m_rep->subClass() == nullptr);
+            QVERIFY(m_rep->tracks() == nullptr);
+        }
+        auto reply = m_rep->start();
+        QVERIFY(reply.waitForFinished());
+
+        QSignalSpy advanceSpy(m_rep.data(), SIGNAL(advance()));
+        QVERIFY(advanceSpy.wait());
+        if (objectMode == QLatin1Literal("ObjectPointer")) {
+            QVERIFY(m_rep->subClass() != nullptr);
+            QCOMPARE(m_rep->subClass()->myPOD(), initialValue);
+            QVERIFY(m_rep->tracks() != nullptr);
+        } else {
+            QVERIFY(m_rep->subClass() == nullptr);
+            QVERIFY(m_rep->tracks() == nullptr);
+        }
+    }
+
+    void cleanupTestCase()
+    {
+        auto reply = m_rep->quit();
+        QVERIFY(reply.waitForFinished());
+    }
+
+private:
+    QScopedPointer<QRemoteObjectNode> m_repNode;
+    QScopedPointer<ParentClassReplica> m_rep;
+    bool serverRestarted = false;
+};
+
+QTEST_MAIN(tst_Client_Process)
+
+#include "main.moc"
diff --git a/tests/auto/restart/restart.pro b/tests/auto/restart/restart.pro
new file mode 100644
index 0000000..0e6ecf4
--- /dev/null
+++ b/tests/auto/restart/restart.pro
@@ -0,0 +1,2 @@
+TEMPLATE = subdirs
+SUBDIRS = client server tst
diff --git a/tests/auto/restart/server/main.cpp b/tests/auto/restart/server/main.cpp
new file mode 100644
index 0000000..b65dc17
--- /dev/null
+++ b/tests/auto/restart/server/main.cpp
@@ -0,0 +1,92 @@
+/****************************************************************************
+**
+** Copyright (C) 2019 Ford Motor Company
+** Contact: https://www.qt.io/licensing/
+**
+** This file is part of the QtRemoteObjects module of the Qt Toolkit.
+**
+** $QT_BEGIN_LICENSE:GPL-EXCEPT$
+** Commercial License Usage
+** Licensees holding valid commercial Qt licenses may use this file in
+** accordance with the commercial license agreement provided with the
+** Software or, alternatively, in accordance with the terms contained in
+** a written agreement between you and The Qt Company. For licensing terms
+** and conditions see https://www.qt.io/terms-conditions. For further
+** information use the contact form at https://www.qt.io/contact-us.
+**
+** GNU General Public License Usage
+** Alternatively, this file may be used under the terms of the GNU
+** General Public License version 3 as published by the Free Software
+** Foundation with exceptions as appearing in the file LICENSE.GPL3-EXCEPT
+** included in the packaging of this file. Please review the following
+** information to ensure the GNU General Public License requirements will
+** be met: https://www.gnu.org/licenses/gpl-3.0.html.
+**
+** $QT_END_LICENSE$
+**
+****************************************************************************/
+
+#include "mytestserver.h"
+#include "rep_subclass_source.h"
+
+#include <QCoreApplication>
+#include <QtTest/QtTest>
+
+class tst_Server_Process : public QObject
+{
+    Q_OBJECT
+
+private Q_SLOTS:
+    void testRun()
+    {
+        const auto runMode = qEnvironmentVariable("RunMode");
+        const auto objectMode = qEnvironmentVariable("ObjectMode");
+
+        QRemoteObjectHost srcNode(QUrl(QStringLiteral("tcp://127.0.0.1:65217")));
+        MyTestServer myTestServer;
+        SubClassSimpleSource subclass;
+        QStringListModel model;
+        const MyPOD initialValue(42, 3.14f, QStringLiteral("SubClass"));
+        if (objectMode == QLatin1Literal("ObjectPointer")) {
+            subclass.setMyPOD(initialValue);
+            model.setStringList(QStringList() << "Track1" << "Track2" << "Track3");
+            myTestServer.setSubClass(&subclass);
+            myTestServer.setTracks(&model);
+        }
+        srcNode.enableRemoting(&myTestServer);
+
+        qDebug() << "Waiting for incoming connections";
+
+        QSignalSpy waitForStartedSpy(&myTestServer, SIGNAL(startedChanged(bool)));
+        QVERIFY(waitForStartedSpy.isValid());
+        QVERIFY(waitForStartedSpy.wait());
+        QCOMPARE(waitForStartedSpy.value(0).value(0).toBool(), true);
+
+        // wait for delivery of events
+        QTest::qWait(200);
+
+        qDebug() << "Client connected";
+        if (runMode != QLatin1Literal("Baseline")) {
+            qDebug() << "Server quitting" << runMode;
+            if (runMode == QLatin1Literal("ServerRestartFatal"))
+                qFatal("Fatal");
+            QCoreApplication::exit();
+            return;
+        }
+        emit myTestServer.advance();
+
+        // wait for quit
+        bool quit = false;
+        connect(&myTestServer, &MyTestServer::quitApp, [&quit]{quit = true;});
+        QTRY_VERIFY_WITH_TIMEOUT(quit, 5000);
+
+        // wait for delivery of events
+        QTest::qWait(200);
+
+        qDebug() << "Done. Shutting down.";
+    }
+};
+
+QTEST_MAIN(tst_Server_Process)
+
+#include "main.moc"
diff --git a/tests/auto/restart/server/mytestserver.cpp b/tests/auto/restart/server/mytestserver.cpp
new file mode 100644
index 0000000..b0c75ab
--- /dev/null
+++ b/tests/auto/restart/server/mytestserver.cpp
@@ -0,0 +1,55 @@
+/****************************************************************************
+**
+** Copyright (C) 2019 Ford Motor Company
+** Contact: https://www.qt.io/licensing/
+**
+** This file is part of the QtRemoteObjects module of the Qt Toolkit.
+**
+** $QT_BEGIN_LICENSE:GPL-EXCEPT$
+** Commercial License Usage
+** Licensees holding valid commercial Qt licenses may use this file in
+** accordance with the commercial license agreement provided with the
+** Software or, alternatively, in accordance with the terms contained in
+** a written agreement between you and The Qt Company. For licensing terms
+** and conditions see https://www.qt.io/terms-conditions. For further
+** information use the contact form at https://www.qt.io/contact-us.
+**
+** GNU General Public License Usage
+** Alternatively, this file may be used under the terms of the GNU
+** General Public License version 3 as published by the Free Software
+** Foundation with exceptions as appearing in the file LICENSE.GPL3-EXCEPT
+** included in the packaging of this file. Please review the following
+** information to ensure the GNU General Public License requirements will
+** be met: https://www.gnu.org/licenses/gpl-3.0.html.
+**
+** $QT_END_LICENSE$
+**
+****************************************************************************/
+
+#include <qdebug.h>
+
+#include "mytestserver.h"
+#include "rep_subclass_source.h"
+
+MyTestServer::MyTestServer(QObject *parent)
+    : ParentClassSimpleSource(parent)
+{
+    qDebug() << "Server started";
+}
+
+MyTestServer::~MyTestServer()
+{
+    qDebug() << "Server stopped";
+}
+
+bool MyTestServer::start()
+{
+    setStarted(true);
+    return true;
+}
+
+bool MyTestServer::quit()
+{
+    emit quitApp();
+    return true;
+}
diff --git a/tests/auto/restart/server/mytestserver.h b/tests/auto/restart/server/mytestserver.h
new file mode 100644
index 0000000..df044c1
--- /dev/null
+++ b/tests/auto/restart/server/mytestserver.h
@@ -0,0 +1,55 @@
+/****************************************************************************
+**
+** Copyright (C) 2019 Ford Motor Company
+** Contact: https://www.qt.io/licensing/
+**
+** This file is part of the QtRemoteObjects module of the Qt Toolkit.
+**
+** $QT_BEGIN_LICENSE:GPL-EXCEPT$
+** Commercial License Usage
+** Licensees holding valid commercial Qt licenses may use this file in
+** accordance with the commercial license agreement provided with the
+** Software or, alternatively, in accordance with the terms contained in
+** a written agreement between you and The Qt Company. For licensing terms
+** and conditions see https://www.qt.io/terms-conditions. For further
+** information use the contact form at https://www.qt.io/contact-us.
+**
+** GNU General Public License Usage
+** Alternatively, this file may be used under the terms of the GNU
+** General Public License version 3 as published by the Free Software
+** Foundation with exceptions as appearing in the file LICENSE.GPL3-EXCEPT
+** included in the packaging of this file. Please review the following
+** information to ensure the GNU General Public License requirements will
+** be met: https://www.gnu.org/licenses/gpl-3.0.html.
+**
+** $QT_END_LICENSE$
+**
+****************************************************************************/
+
+#ifndef MYTESTSERVER_H
+#define MYTESTSERVER_H
+
+#include <QTimer>
+
+#include <QtRemoteObjects/qremoteobjectnode.h>
+#include <QtRemoteObjects/qremoteobjectsource.h>
+
+#include "rep_subclass_source.h"
+
+class MyTestServer : public ParentClassSimpleSource
+{
+    Q_OBJECT
+
+public:
+    MyTestServer(QObject *parent = nullptr);
+    ~MyTestServer() override;
+
+public Q_SLOTS:
+    bool start() override;
+    bool quit() override;
+
+Q_SIGNALS:
+    void quitApp();
+};
+
+#endif // MYTESTSERVER_H
diff --git a/tests/auto/restart/server/server.pro b/tests/auto/restart/server/server.pro
new file mode 100644
index 0000000..eefc7a5
--- /dev/null
+++ b/tests/auto/restart/server/server.pro
@@ -0,0 +1,19 @@
+TEMPLATE = app
+QT       += remoteobjects core testlib
+QT       -= gui
+
+TARGET = server
+DESTDIR = ./
+CONFIG   += c++11
+CONFIG   -= app_bundle
+
+REPC_SOURCE = $$PWD/../subclass.rep
+
+SOURCES += main.cpp \
+    mytestserver.cpp
+
+HEADERS += \
+    mytestserver.h
+    $$OUT_PWD/rep_subclass_source.h
+
+INCLUDEPATH += $$PWD
diff --git a/tests/auto/restart/subclass.rep b/tests/auto/restart/subclass.rep
new file mode 100644
index 0000000..1ebbd6e
--- /dev/null
+++ b/tests/auto/restart/subclass.rep
@@ -0,0 +1,19 @@
+POD MyPOD(int i, float f, QString s)
+
+class SubClass
+{
+    PROP(MyPOD myPOD)
+}
+
+class ParentClass
+{
+    PROP(bool started = false)
+
+    SLOT(bool start())
+    SLOT(bool quit())
+    SIGNAL(advance())
+
+    CLASS subClass(SubClass)
+    MODEL tracks(display)
+}
+
diff --git a/tests/auto/restart/tst/tst.pro b/tests/auto/restart/tst/tst.pro
new file mode 100644
index 0000000..d45e701
--- /dev/null
+++ b/tests/auto/restart/tst/tst.pro
@@ -0,0 +1,8 @@
+CONFIG += testcase c++11
+CONFIG -= app_bundle
+TARGET = tst_restart
+DESTDIR = ./
+QT += testlib remoteobjects
+QT -= gui
+
+SOURCES += tst_restart.cpp
diff --git a/tests/auto/restart/tst/tst_restart.cpp b/tests/auto/restart/tst/tst_restart.cpp
new file mode 100644
index 0000000..a3633fa
--- /dev/null
+++ b/tests/auto/restart/tst/tst_restart.cpp
@@ -0,0 +1,142 @@
+/****************************************************************************
+**
+** Copyright (C) 2019 Ford Motor Company
+** Contact: https://www.qt.io/licensing/
+**
+** This file is part of the QtRemoteObjects module of the Qt Toolkit.
+**
+** $QT_BEGIN_LICENSE:GPL-EXCEPT$
+** Commercial License Usage
+** Licensees holding valid commercial Qt licenses may use this file in
+** accordance with the commercial license agreement provided with the
+** Software or, alternatively, in accordance with the terms contained in
+** a written agreement between you and The Qt Company. For licensing terms
+** and conditions see https://www.qt.io/terms-conditions. For further
+** information use the contact form at https://www.qt.io/contact-us.
+**
+** GNU General Public License Usage
+** Alternatively, this file may be used under the terms of the GNU
+** General Public License version 3 as published by the Free Software
+** Foundation with exceptions as appearing in the file LICENSE.GPL3-EXCEPT
+** included in the packaging of this file. Please review the following
+** information to ensure the GNU General Public License requirements will
+** be met: https://www.gnu.org/licenses/gpl-3.0.html.
+**
+** $QT_END_LICENSE$
+**
+****************************************************************************/
+
+#include <QtTest/QtTest>
+#include <QMetaType>
+#include <QProcess>
+#include <QStandardPaths>
+
+namespace {
+
+QString findExecutable(const QString &executableName, const QStringList &paths)
+{
+    const auto path = QStandardPaths::findExecutable(executableName, paths);
+    if (!path.isEmpty()) {
+        return path;
+    }
+
+    qWarning() << "Could not find executable:" << executableName << "in any of" << paths;
+    return QString();
+}
+
+}
+
+class tst_Restart: public QObject
+{
+    Q_OBJECT
+
+public:
+    enum RunMode { Baseline, ServerRestartGraceful, ServerRestartFatal };
+    enum ObjectMode { NullPointer, ObjectPointer };
+    Q_ENUM(RunMode)
+    Q_ENUM(ObjectMode)
+
+private slots:
+    void initTestCase()
+    {
+        QLoggingCategory::setFilterRules("qt.remoteobjects.warning=false");
+    }
+
+    void cleanup()
+    {
+        // wait for delivery of RemoveObject events to the source
+        QTest::qWait(200);
+    }
+
+    void testRun_data()
+    {
+        QTest::addColumn<RunMode>("runMode");
+        QTest::addColumn<ObjectMode>("objectMode");
+        auto runModeMeta = QMetaEnum::fromType<RunMode>();
+        auto objectModeMeta = QMetaEnum::fromType<ObjectMode>();
+        for (int i = 0; i < runModeMeta.keyCount(); i++) {
+            for (int j = 0; j < objectModeMeta.keyCount(); j++) {
+                auto ba = QByteArray(runModeMeta.valueToKey(i));
+                ba = ba.append("_").append(objectModeMeta.valueToKey(j));
+                QTest::newRow(ba.data()) << static_cast<RunMode>(i) << static_cast<ObjectMode>(j);
+            }
+        }
+    }
+
+    void testRun()
+    {
+        QFETCH(RunMode, runMode);
+        QFETCH(ObjectMode, objectMode);
+
+        qDebug() << "Starting server process" << runMode;
+        bool serverRestart = runMode == ServerRestartFatal || runMode == ServerRestartGraceful;
+        QProcess serverProc;
+        serverProc.setProcessChannelMode(QProcess::ForwardedChannels);
+        QProcessEnvironment env = QProcessEnvironment::systemEnvironment();
+        env.insert("RunMode", QVariant::fromValue(runMode).toString());
+        env.insert("ObjectMode", QVariant::fromValue(objectMode).toString());
+        serverProc.setProcessEnvironment(env);
+        serverProc.start(findExecutable("server", {
+            QCoreApplication::applicationDirPath() + "/../server/"
+        }));
+        QVERIFY(serverProc.waitForStarted());
+
+        // wait for server start
+        QTest::qWait(200);
+
+        qDebug() << "Starting client process";
+        QProcess clientProc;
+        clientProc.setProcessChannelMode(QProcess::ForwardedChannels);
+        clientProc.setProcessEnvironment(env);
+        clientProc.start(findExecutable("client", {
+            QCoreApplication::applicationDirPath() + "/../client/"
+        }));
+        QVERIFY(clientProc.waitForStarted());
+
+        if (serverRestart) {
+            env.insert("RunMode", QVariant::fromValue(Baseline).toString()); // Don't include ServerRestart environment variable this time
+            qDebug() << "Waiting for server exit";
+            QVERIFY(serverProc.waitForFinished());
+            if (runMode == ServerRestartFatal)
+                QVERIFY(serverProc.exitCode() != 0);
+            else
+                QCOMPARE(serverProc.exitCode(), 0);
+            qDebug() << "Restarting server";
+            serverProc.setProcessEnvironment(env);
+            serverProc.start(findExecutable("server", {
+                QCoreApplication::applicationDirPath() + "/../server/"
+            }));
+            QVERIFY(serverProc.waitForStarted());
+        }
+
+        QVERIFY(clientProc.waitForFinished());
+        QVERIFY(serverProc.waitForFinished());
+
+        QCOMPARE(serverProc.exitCode(), 0);
+        QCOMPARE(clientProc.exitCode(), 0);
+    }
+};
+
+QTEST_MAIN(tst_Restart)
+
+#include "tst_restart.moc"