Set parent of internal socket objects

After moving the websocket into another thread
current code doesn't work because then the QTcpSocket/QSslSocket
objects reside in a different thread, for instance:
"QSocketNotifier: Socket notifiers cannot be enabled or disabled from another thread"

QObject::moveToThread(QThread*) also moves QObjects's children,
therefore their internal socket objects need to be children of QWebSocket.

QWebSocket has ownership of the internal socket, and the
smart pointer is not needed any more.

Change of cleanup code to prevent crashes with clang/msvc builds:
QWebSocketPrivate is a scoped member of QObject (not QWebSocket)
and is destroyed after QObject destructor body was executed,
and so  m_pSocket&co had already been destroyed (being children)
when the destructor of QWebSocketPrivate is called via the
scoped pointer.

Analogous to 64927e04f202d33b9a9a1f94141ef692c0b513ac

Change-Id: I1ade6cda3fa793c30332cc5e103025e2dda3c78c
Reviewed-by: Luca Niccoli <lultimouomo@gmail.com>
Reviewed-by: Alex Blasche <alexander.blasche@theqtcompany.com>

4 files changed, 132 insertions, 19 deletions

diff --git a/src/websockets/qwebsocket.cpp b/src/websockets/qwebsocket.cpp
index a77f23c..ee9b2d2 100644
--- a/src/websockets/qwebsocket.cpp
+++ b/src/websockets/qwebsocket.cpp
@@ -281,6 +281,8 @@ QWebSocket::QWebSocket(const QString &origin,
  */
 QWebSocket::~QWebSocket()
 {
+    Q_D(QWebSocket);
+    d->closeGoingAway();
 }
 
 /*!
diff --git a/src/websockets/qwebsocket_p.cpp b/src/websockets/qwebsocket_p.cpp
index 174214c..2a27abb 100644
--- a/src/websockets/qwebsocket_p.cpp
+++ b/src/websockets/qwebsocket_p.cpp
@@ -84,7 +84,7 @@ QWebSocketPrivate::QWebSocketPrivate(const QString &origin, QWebSocketProtocol::
                                      QWebSocket *pWebSocket) :
     QObjectPrivate(),
     q_ptr(pWebSocket),
-    m_pSocket(),
+    m_pSocket(Q_NULLPTR),
     m_errorString(),
     m_version(version),
     m_resourceName(),
@@ -154,7 +154,7 @@ void QWebSocketPrivate::init()
     m_pMaskGenerator->seed();
 
     if (m_pSocket) {
-        makeConnections(m_pSocket.data());
+        makeConnections(m_pSocket);
     }
 }
 
@@ -163,11 +163,18 @@ void QWebSocketPrivate::init()
 */
 QWebSocketPrivate::~QWebSocketPrivate()
 {
+}
+
+/*!
+    \internal
+*/
+void QWebSocketPrivate::closeGoingAway()
+{
     if (!m_pSocket)
         return;
     if (state() == QAbstractSocket::ConnectedState)
         close(QWebSocketProtocol::CloseCodeGoingAway, QWebSocket::tr("Connection closed"));
-    releaseConnections(m_pSocket.data());
+    releaseConnections(m_pSocket);
 }
 
 /*!
@@ -262,7 +269,7 @@ void QWebSocketPrivate::ignoreSslErrors()
 {
     m_configuration.m_ignoreSslErrors = true;
     if (Q_LIKELY(m_pSocket)) {
-        QSslSocket *pSslSocket = qobject_cast<QSslSocket *>(m_pSocket.data());
+        QSslSocket *pSslSocket = qobject_cast<QSslSocket *>(m_pSocket);
         if (Q_LIKELY(pSslSocket))
             pSslSocket->ignoreSslErrors();
     }
@@ -334,17 +341,17 @@ void QWebSocketPrivate::open(const QUrl &url, bool mask)
 {
     //just delete the old socket for the moment;
     //later, we can add more 'intelligent' handling by looking at the URL
-    //m_pSocket.reset();
+
     Q_Q(QWebSocket);
     if (!url.isValid() || url.toString().contains(QStringLiteral("\r\n"))) {
         setErrorString(QWebSocket::tr("Invalid URL."));
         Q_EMIT q->error(QAbstractSocket::ConnectionRefusedError);
         return;
     }
-    QTcpSocket *pTcpSocket = m_pSocket.take();
-    if (pTcpSocket) {
-        releaseConnections(pTcpSocket);
-        pTcpSocket->deleteLater();
+    if (m_pSocket) {
+        releaseConnections(m_pSocket);
+        m_pSocket->deleteLater();
+        m_pSocket = Q_NULLPTR;
     }
     //if (m_url != url)
     if (Q_LIKELY(!m_pSocket)) {
@@ -380,15 +387,15 @@ void QWebSocketPrivate::open(const QUrl &url, bool mask)
                 setErrorString(message);
                 Q_EMIT q->error(QAbstractSocket::UnsupportedSocketOperationError);
             } else {
-                QSslSocket *sslSocket = new QSslSocket;
-                m_pSocket.reset(sslSocket);
+                QSslSocket *sslSocket = new QSslSocket(q_ptr);
+                m_pSocket = sslSocket;
                 if (Q_LIKELY(m_pSocket)) {
                     m_pSocket->setSocketOption(QAbstractSocket::LowDelayOption, 1);
                     m_pSocket->setSocketOption(QAbstractSocket::KeepAliveOption, 1);
                     m_pSocket->setReadBufferSize(m_readBufferSize);
                     m_pSocket->setPauseMode(m_pauseMode);
 
-                    makeConnections(m_pSocket.data());
+                    makeConnections(m_pSocket);
                     setSocketState(QAbstractSocket::ConnectingState);
 
                     sslSocket->setSslConfiguration(m_configuration.m_sslConfiguration);
@@ -409,14 +416,14 @@ void QWebSocketPrivate::open(const QUrl &url, bool mask)
         } else
     #endif
         if (url.scheme() == QStringLiteral("ws")) {
-            m_pSocket.reset(new QTcpSocket);
+            m_pSocket = new QTcpSocket(q_ptr);
             if (Q_LIKELY(m_pSocket)) {
                 m_pSocket->setSocketOption(QAbstractSocket::LowDelayOption, 1);
                 m_pSocket->setSocketOption(QAbstractSocket::KeepAliveOption, 1);
                 m_pSocket->setReadBufferSize(m_readBufferSize);
                 m_pSocket->setPauseMode(m_pauseMode);
 
-                makeConnections(m_pSocket.data());
+                makeConnections(m_pSocket);
                 setSocketState(QAbstractSocket::ConnectingState);
     #ifndef QT_NO_NETWORKPROXY
                 m_pSocket->setProxy(m_configuration.m_proxy);
@@ -1091,8 +1098,8 @@ void QWebSocketPrivate::processStateChanged(QAbstractSocket::SocketState socketS
 void QWebSocketPrivate::socketDestroyed(QObject *socket)
 {
     Q_ASSERT(m_pSocket);
-    if (m_pSocket.data() == socket)
-        m_pSocket.take();
+    if (m_pSocket == socket)
+        m_pSocket = Q_NULLPTR;
 }
 
 /*!
@@ -1103,9 +1110,9 @@ void QWebSocketPrivate::processData()
     Q_ASSERT(m_pSocket);
     while (m_pSocket->bytesAvailable()) {
         if (state() == QAbstractSocket::ConnectingState)
-            processHandshake(m_pSocket.data());
+            processHandshake(m_pSocket);
         else
-            m_dataProcessor.process(m_pSocket.data());
+            m_dataProcessor.process(m_pSocket);
     }
 }
 
diff --git a/src/websockets/qwebsocket_p.h b/src/websockets/qwebsocket_p.h
index 7379c51..99acad2 100644
--- a/src/websockets/qwebsocket_p.h
+++ b/src/websockets/qwebsocket_p.h
@@ -143,6 +143,7 @@ public:
     QSslConfiguration sslConfiguration() const;
 #endif
 
+    void closeGoingAway();
     void close(QWebSocketProtocol::CloseCode closeCode, QString reason);
     void open(const QUrl &url, bool mask);
     void ping(const QByteArray &payload);
@@ -196,7 +197,7 @@ private:
     qint64 writeFrames(const QList<QByteArray> &frames) Q_REQUIRED_RESULT;
     qint64 writeFrame(const QByteArray &frame) Q_REQUIRED_RESULT;
 
-    QScopedPointer<QTcpSocket> m_pSocket;
+    QTcpSocket *m_pSocket;
     QString m_errorString;
     QWebSocketProtocol::Version m_version;
     QUrl m_resource;
diff --git a/tests/auto/websockets/qwebsocket/tst_qwebsocket.cpp b/tests/auto/websockets/qwebsocket/tst_qwebsocket.cpp
index aca25d0..cc3bca6 100644
--- a/tests/auto/websockets/qwebsocket/tst_qwebsocket.cpp
+++ b/tests/auto/websockets/qwebsocket/tst_qwebsocket.cpp
@@ -140,6 +140,8 @@ private Q_SLOTS:
     void tst_sendTextMessage();
     void tst_sendBinaryMessage();
     void tst_errorString();
+    void tst_moveToThread();
+    void tst_moveToThreadNoWarning();
 #ifndef QT_NO_NETWORKPROXY
     void tst_setProxy();
 #endif
@@ -581,6 +583,107 @@ void tst_QWebSocket::tst_errorString()
     QCOMPARE(socket.errorString(), QStringLiteral("Host not found"));
 }
 
+class WebSocket : public QWebSocket
+{
+    Q_OBJECT
+
+public:
+    explicit WebSocket()
+    {
+        connect(this, SIGNAL(triggerClose()), SLOT(onClose()), Qt::QueuedConnection);
+        connect(this, SIGNAL(triggerOpen(QUrl)), SLOT(onOpen(QUrl)), Qt::QueuedConnection);
+        connect(this, SIGNAL(triggerSendTextMessage(QString)), SLOT(onSendTextMessage(QString)), Qt::QueuedConnection);
+        connect(this, SIGNAL(textMessageReceived(QString)), this, SLOT(onTextMessageReceived(QString)), Qt::QueuedConnection);
+    }
+
+    void asyncClose() { triggerClose(); }
+    void asyncOpen(const QUrl &url) { triggerOpen(url); }
+    void asyncSendTextMessage(const QString &msg) { triggerSendTextMessage(msg); }
+
+    QString receivedMessage;
+
+Q_SIGNALS:
+    void triggerClose();
+    void triggerOpen(const QUrl &);
+    void triggerSendTextMessage(const QString &);
+    void done();
+
+private Q_SLOTS:
+    void onClose() { close(); }
+    void onOpen(const QUrl &url) { open(url); }
+    void onSendTextMessage(const QString &msg) { sendTextMessage(msg); }
+    void onTextMessageReceived(const QString &msg) { receivedMessage = msg; done(); }
+};
+
+struct Warned
+{
+    static QtMessageHandler origHandler;
+    static bool warned;
+    static void messageHandler(QtMsgType type, const QMessageLogContext& context, const QString& str)
+    {
+        if (type == QtWarningMsg) {
+            warned = true;
+        }
+        if (origHandler)
+            origHandler(type, context, str);
+    }
+};
+QtMessageHandler Warned::origHandler = 0;
+bool Warned::warned = false;
+
+
+void tst_QWebSocket::tst_moveToThread()
+{
+    Warned::origHandler = qInstallMessageHandler(&Warned::messageHandler);
+
+    EchoServer echoServer;
+
+    QThread* thread = new QThread;
+    thread->start();
+
+    WebSocket* socket = new WebSocket;
+    socket->moveToThread(thread);
+
+    const QString textMessage = QStringLiteral("Hello world!");
+    QSignalSpy socketConnectedSpy(socket, SIGNAL(connected()));
+    QUrl url = QUrl(QStringLiteral("ws://") + echoServer.hostAddress().toString() +
+                    QStringLiteral(":") + QString::number(echoServer.port()));
+    url.setPath("/segment/with spaces");
+    url.addQueryItem("queryitem", "with encoded characters");
+
+    socket->asyncOpen(url);
+    if (socketConnectedSpy.count() == 0)
+        QVERIFY(socketConnectedSpy.wait(500));
+
+    socket->asyncSendTextMessage(textMessage);
+
+    QTimer timer;
+    timer.setInterval(1000);
+    timer.start();
+    QEventLoop loop;
+    connect(socket, SIGNAL(done()), &loop, SLOT(quit()));
+    connect(socket, SIGNAL(done()), &timer, SLOT(stop()));
+    connect(&timer, SIGNAL(timeout()), &loop, SLOT(quit()));
+    loop.exec();
+
+    socket->asyncClose();
+
+    QCOMPARE(timer.isActive(), false);
+    QCOMPARE(socket->receivedMessage, textMessage);
+
+    socket->deleteLater();
+    thread->quit();
+    thread->deleteLater();
+}
+
+void tst_QWebSocket::tst_moveToThreadNoWarning()
+{
+    // check for warnings in tst_moveToThread()
+    // couldn't done there because warnings are processed after the test run
+    QCOMPARE(Warned::warned, false);
+}
+
+
 #ifndef QT_NO_NETWORKPROXY
 void tst_QWebSocket::tst_setProxy()
 {