Fix mixing up raw-xml requests in the same server object

Create a server object for each incoming connection.

Fixes #288

docs/CHANGES_2_3.txt

@@ -9,4 +9,5 @@ Client-side:
 
 Server-side:
 ============
-*
+* To avoid mixing up raw-xml requests in the same server object (#288), KDSoap now creates a server object for each incoming connection.
+  Make sure your server object is ready to be created multiple times (this was already a requirement when enabling multi-threading with setThreadPool()).

src/KDSoapServer/KDSoapServerSocket.cpp

@@ -54,6 +54,8 @@ KDSoapServerSocket::~KDSoapServerSocket()
 {
     // same as m_owner->socketDeleted, but safe in case m_owner is deleted first
     emit socketDeleted(this);
+
+    delete m_serverObject;
 }
 
 typedef QMap<QByteArray, QByteArray> HeadersMap;

src/KDSoapServer/KDSoapSocketList.cpp

@@ -14,21 +14,18 @@
 
 KDSoapSocketList::KDSoapSocketList(KDSoapServer *server)
     : m_server(server)
-    , m_serverObject(server->createServerObject())
     , m_totalConnectionCount(0)
 {
     Q_ASSERT(m_server);
-    Q_ASSERT(m_serverObject);
 }
 
 KDSoapSocketList::~KDSoapSocketList()
 {
-    delete m_serverObject;
 }
 
 KDSoapServerSocket *KDSoapSocketList::handleIncomingConnection(int socketDescriptor)
 {
-    KDSoapServerSocket *socket = new KDSoapServerSocket(this, m_serverObject);
+    KDSoapServerSocket *socket = new KDSoapServerSocket(this, m_server->createServerObject());
     socket->setSocketDescriptor(socketDescriptor);
 
 #ifndef QT_NO_SSL

src/KDSoapServer/KDSoapSocketList_p.h

@@ -45,7 +45,6 @@ public Q_SLOTS:
 
 private:
     KDSoapServer *m_server;
-    QObject *m_serverObject;
     QSet<KDSoapServerSocket *> m_sockets;
     QAtomicInt m_totalConnectionCount;
 };

unittests/serverlib/test_serverlib.cpp

@@ -39,8 +39,8 @@ Q_DECLARE_METATYPE(QFile::Permissions)
 static const char *myWsdlNamespace = "http://www.kdab.com/xml/MyWsdl/";
 
 class CountryServerObject;
-typedef QMap<QThread *, CountryServerObject *> ServerObjectsMap;
-ServerObjectsMap s_serverObjects;
+typedef QList<CountryServerObject *> ServerObjectsList;
+ServerObjectsList s_serverObjects;
 QMutex s_serverObjectsMutex;
 
 class PublicThread : public QThread
@@ -88,13 +88,13 @@ class CountryServerObject : public QObject,
     {
         // qDebug() << "Server object created in thread" << QThread::currentThread();
         QMutexLocker locker(&s_serverObjectsMutex);
-        s_serverObjects.insert(QThread::currentThread(), this);
+        s_serverObjects.append(this);
     }
     ~CountryServerObject()
     {
         QMutexLocker locker(&s_serverObjectsMutex);
-        Q_ASSERT(s_serverObjects.value(QThread::currentThread()) == this);
-        s_serverObjects.remove(QThread::currentThread());
+        Q_ASSERT(s_serverObjects.contains(this));
+        s_serverObjects.removeOne(this);
     }
 
     virtual void processRequest(const KDSoapMessage &request, KDSoapMessage &response, const QByteArray &soapAction) override;
@@ -189,7 +189,7 @@ class CountryServerObject : public QObject,
     {
         // Should be called in same thread as constructor
         s_serverObjectsMutex.lock();
-        Q_ASSERT(s_serverObjects.value(QThread::currentThread()) == this);
+        Q_ASSERT(s_serverObjects.contains(this));
         s_serverObjectsMutex.unlock();
         if (employeeName.isEmpty()) {
             setFault(QLatin1String("Client.Data"), QLatin1String("Empty employee name"), QLatin1String("CountryServerObject"),
@@ -403,7 +403,7 @@ private Q_SLOTS:
             QCOMPARE(response.childValues().first().value().toString(), expectedCountry());
 
             QCOMPARE(s_serverObjects.count(), 1);
-            QVERIFY(s_serverObjects.value(&serverThread)); // request handled by server thread itself (no thread pool)
+            QCOMPARE(s_serverObjects.at(0)->thread(), &serverThread); // request handled by server thread itself (no thread pool)
             QCOMPARE(server->totalConnectionCount(), 1);
             delete client;
             QTest::qWait(100);
@@ -533,7 +533,7 @@ private Q_SLOTS:
             const KDSoapMessage response = client->call(QLatin1String("getEmployeeCountry"), countryMessage());
             QCOMPARE(response.childValues().first().value().toString(), expectedCountry());
             QCOMPARE(s_serverObjects.count(), 1);
-            QThread *thread = s_serverObjects.begin().key();
+            QThread *thread = s_serverObjects.at(0)->thread();
             QVERIFY(thread != qApp->thread());
             QVERIFY(thread != &serverThread);
             QCOMPARE(server->totalConnectionCount(), 1);
@@ -547,7 +547,7 @@ private Q_SLOTS:
         QTest::addColumn<int>("maxThreads");
         QTest::addColumn<int>("numRequests");
         QTest::addColumn<int>("numClients");
-        QTest::addColumn<int>("expectedServerObjects");
+        QTest::addColumn<int>("expectedThreads");
 
         // QNetworkAccessManager only does 6 concurrent http requests
         // (QHttpNetworkConnectionPrivate::defaultHttpChannelCount = 6)
@@ -564,7 +564,7 @@ private Q_SLOTS:
         QFETCH(int, maxThreads);
         QFETCH(int, numRequests);
         QFETCH(int, numClients);
-        QFETCH(int, expectedServerObjects);
+        QFETCH(int, expectedThreads);
         {
             KDSoapThreadPool threadPool;
             threadPool.setMaxThreadCount(maxThreads);
@@ -586,13 +586,15 @@ private Q_SLOTS:
                 for (const KDSoapMessage &response : std::as_const(m_returnMessages)) {
                     QCOMPARE(response.childValues().first().value().toString(), expectedCountry());
                 }
-                QCOMPARE(s_serverObjects.count(), expectedServerObjects);
-                QMapIterator<QThread *, CountryServerObject *> it(s_serverObjects);
-                while (it.hasNext()) {
-                    QThread *thread = it.next().key();
+                QCOMPARE(s_serverObjects.count(), numRequests);
+                QSet<QThread *> usedThreads;
+                for (CountryServerObject *obj : qAsConst(s_serverObjects)) {
+                    QThread *thread = obj->thread();
                     QVERIFY(thread != qApp->thread());
                     QVERIFY(thread != &serverThread);
+                    usedThreads.insert(thread);
                 }
+                QCOMPARE(usedThreads.count(), expectedThreads);
             }
             QCOMPARE(server->totalConnectionCount(), numClients * numRequests);
         }

unittests/wsdl_document/test_wsdl_document.cpp

@@ -1237,6 +1237,7 @@ void WsdlDocumentTest::testDisconnectDuringDelayedCall()
 {
     TestServerThread<DocServer> serverThread;
     DocServer *server = serverThread.startThread();
+    QPointer<DocServerObject> serverObject;
     {
         MyWsdlDocument service;
         service.setEndPoint(server->endPoint());
@@ -1246,14 +1247,14 @@ void WsdlDocumentTest::testDisconnectDuringDelayedCall()
         job->start();
         // Wait until the server method is called
         QTRY_VERIFY(server->lastServerObject());
-        QTRY_COMPARE(server->lastServerObject()->m_lastMethodCalled, QString::fromLatin1("delayedAddEmployee"));
+        serverObject = server->lastServerObject();
+        QTRY_COMPARE(serverObject->m_lastMethodCalled, QString::fromLatin1("delayedAddEmployee"));
 
         delete job;
     } // Disconnect the client
 
-    // Let the server continue
-
-    QTRY_COMPARE(server->lastServerObject()->m_lastMethodCalled, QString::fromLatin1("slotDelayedResponse"));
+    // Now that there's one server object per socket (#289), the server object gets deleted before the delayed response
+    QTRY_VERIFY(serverObject.isNull());
 }
 
 // Same as testSequenceInResponse (thomas-bayer.wsdl), but as a server test, by calling DocServer on a different path