Break socket listen out of QRemoteObjectSourceIo constructorv5.12.0-beta4

The QRemoteObjectSourceIo constructor that accepted a host address
attempted to start listening on that address.  This could result in an
invalid state with no way to propagate the error.

Change-Id: I80c103122794b164788702ba5c37fd10080e9ecd
Reviewed-by: Brett Stottlemyer <bstottle@ford.com>

5 files changed, 48 insertions, 19 deletions

diff --git a/src/remoteobjects/qremoteobjectnode.cpp b/src/remoteobjects/qremoteobjectnode.cpp
index f095e1e..5d86605 100644
--- a/src/remoteobjects/qremoteobjectnode.cpp
+++ b/src/remoteobjects/qremoteobjectnode.cpp
@@ -1337,6 +1337,7 @@ void QRemoteObjectNodePrivate::onClientRead(QObject *obj)
     \value MissingObjectName The given QObject does not have objectName() set.
     \value HostUrlInvalid The given url has an invalid or unrecognized scheme.
     \value ProtocolMismatch The client and the server have different protocol versions.
+    \value ListenFailed Can't listen on the specified host port.
 */
 
 /*!
@@ -1599,6 +1600,13 @@ bool QRemoteObjectHostBase::setHostUrl(const QUrl &hostAddress, AllowedSchemas a
     }
     d->remoteObjectIo = new QRemoteObjectSourceIo(hostAddress, this);
 
+    if (allowedSchemas == AllowedSchemas::BuiltInSchemasOnly && !d->remoteObjectIo->startListening()) {
+        d->setLastError(ListenFailed);
+        delete d->remoteObjectIo;
+        d->remoteObjectIo = nullptr;
+        return false;
+    }
+
     //If we've given a name to the node, set it on the sourceIo as well
     if (!objectName().isEmpty())
         d->remoteObjectIo->setObjectName(objectName());
diff --git a/src/remoteobjects/qremoteobjectnode.h b/src/remoteobjects/qremoteobjectnode.h
index f2d00d6..d69b369 100644
--- a/src/remoteobjects/qremoteobjectnode.h
+++ b/src/remoteobjects/qremoteobjectnode.h
@@ -96,7 +96,8 @@ public:
         SourceNotRegistered,
         MissingObjectName,
         HostUrlInvalid,
-        ProtocolMismatch
+        ProtocolMismatch,
+        ListenFailed
     };
     Q_ENUM(ErrorCode)
 
diff --git a/src/remoteobjects/qremoteobjectsourceio.cpp b/src/remoteobjects/qremoteobjectsourceio.cpp
index d864ed5..c5bf36e 100644
--- a/src/remoteobjects/qremoteobjectsourceio.cpp
+++ b/src/remoteobjects/qremoteobjectsourceio.cpp
@@ -54,21 +54,10 @@ QRemoteObjectSourceIo::QRemoteObjectSourceIo(const QUrl &address, QObject *paren
     : QObject(parent)
     , m_server(QtROServerFactory::instance()->isValid(address) ?
                QtROServerFactory::instance()->create(address, this) : nullptr)
+    , m_address(address)
 {
-    if (m_server && m_server->listen(address)) {
-        qRODebug(this) << "QRemoteObjectSourceIo is Listening" << address;
-    } else {
-        if (m_server) {
-            qROWarning(this) << "Listen failed for URL:" << address;
-            qROWarning(this) << m_server->serverError();
-        } else {
-            m_address = address;
-            qRODebug(this) << "Using" << address << "as external url.";
-        }
-        return;
-    }
-
-    connect(m_server.data(), &QConnectionAbstractServer::newConnection, this, &QRemoteObjectSourceIo::handleConnection);
+    if (m_server == nullptr)
+        qRODebug(this) << "Using" << m_address << "as external url.";
 }
 
 QRemoteObjectSourceIo::QRemoteObjectSourceIo(QObject *parent)
@@ -82,6 +71,20 @@ QRemoteObjectSourceIo::~QRemoteObjectSourceIo()
     qDeleteAll(m_sourceRoots.values());
 }
 
+bool QRemoteObjectSourceIo::startListening()
+{
+    if (!m_server->listen(m_address)) {
+        qROCritical(this) << "Listen failed for URL:" << m_address;
+        qROCritical(this) << m_server->serverError();
+        return false;
+    }
+
+    qRODebug(this) << "QRemoteObjectSourceIo is Listening" << m_address;
+    connect(m_server.data(), &QConnectionAbstractServer::newConnection, this,
+            &QRemoteObjectSourceIo::handleConnection);
+    return true;
+}
+
 bool QRemoteObjectSourceIo::enableRemoting(QObject *object, const QMetaObject *meta, const QString &name, const QString &typeName)
 {
     if (m_sourceRoots.contains(name)) {
diff --git a/src/remoteobjects/qremoteobjectsourceio_p.h b/src/remoteobjects/qremoteobjectsourceio_p.h
index 4cc045c..59f9888 100644
--- a/src/remoteobjects/qremoteobjectsourceio_p.h
+++ b/src/remoteobjects/qremoteobjectsourceio_p.h
@@ -73,6 +73,7 @@ public:
     explicit QRemoteObjectSourceIo(QObject *parent = nullptr);
     ~QRemoteObjectSourceIo() override;
 
+    bool startListening();
     bool enableRemoting(QObject *object, const QMetaObject *meta, const QString &name,
                         const QString &typeName);
     bool enableRemoting(QObject *object, const SourceApiMap *api, QObject *adapter = nullptr);
diff --git a/tests/auto/integration/tst_integration.cpp b/tests/auto/integration/tst_integration.cpp
index 8643002..5c42319 100644
--- a/tests/auto/integration/tst_integration.cpp
+++ b/tests/auto/integration/tst_integration.cpp
@@ -210,14 +210,14 @@ private slots:
         QTest::addColumn<QUrl>("hostUrl");
         QTest::addColumn<QUrl>("registryUrl");
 
-#ifndef SKIP_LOCAL
-        QTest::newRow("local") << QUrl(QLatin1String("local:replica_local_integration")) << QUrl(QLatin1String("local:registry_local_integration"));
-#endif
         QTest::newRow("tcp") << QUrl(QLatin1String("tcp://127.0.0.1:65511")) << QUrl(QLatin1String("tcp://127.0.0.1:65512"));
-        QTest::newRow("external") << QUrl() << QUrl();
 #ifdef __QNXNTO__
         QTest::newRow("qnx") << QUrl(QLatin1String("qnx:replica")) << QUrl(QLatin1String("qnx:registry"));
 #endif
+#ifndef SKIP_LOCAL
+        QTest::newRow("local") << QUrl(QLatin1String("local:replica_local_integration")) << QUrl(QLatin1String("local:registry_local_integration"));
+#endif
+        QTest::newRow("external") << QUrl() << QUrl();
     }
 
     void initTestCase()
@@ -1368,6 +1368,22 @@ private slots:
         QVERIFY(testServer.waitForFinished());
     }
 #endif
+
+    void tcpListenFailedTest()
+    {
+        QFETCH_GLOBAL(QUrl, registryUrl);
+
+        if (registryUrl.scheme() != QRemoteObjectStringLiterals::tcp())
+            QSKIP("Skipping test for local and external backends.");
+
+        // Need the Host or Registry running so that the port is in use.
+        setupRegistry();
+        QRemoteObjectHost badHost;
+        badHost.setHostUrl(registryUrl);
+        QCOMPARE(badHost.lastError(), QRemoteObjectNode::ListenFailed);
+
+    }
+
 };
 
 QTEST_MAIN(tst_Integration)