Renderer: remove code shutting down thread in renderer dtor

This should have already been performed by Renderer::shutdown(). Doing it
again in the dtor only hides potential issues.

Change-Id: I33b02df42d1667ea4cbc02cb6f4cb8fabcf70996
Reviewed-by: Mike Krus <mike.krus@kdab.com>

8 files changed, 248 insertions, 5 deletions

diff --git a/src/render/renderers/opengl/renderer/renderer.cpp b/src/render/renderers/opengl/renderer/renderer.cpp
index 7319fd0e5..c9ba6409d 100644
--- a/src/render/renderers/opengl/renderer/renderer.cpp
+++ b/src/render/renderers/opengl/renderer/renderer.cpp
@@ -243,11 +243,9 @@ Renderer::Renderer(QRenderAspect::RenderType type)
 
 Renderer::~Renderer()
 {
-    // If using a threaded rendering approach, tell the thread to exit
-    // and wait for it to be done
-    m_running.fetchAndStoreOrdered(0);
+    Q_ASSERT(m_running.fetchAndStoreOrdered(0) == 0);
     if (m_renderThread)
-        m_renderThread->wait();
+        Q_ASSERT(m_renderThread->isFinished());
 
     delete m_renderQueue;
     delete m_defaultRenderStateSet;
diff --git a/tests/auto/auto.pro b/tests/auto/auto.pro
index 6bb3597ea..971edcb39 100644
--- a/tests/auto/auto.pro
+++ b/tests/auto/auto.pro
@@ -3,7 +3,8 @@ TEMPLATE = subdirs
 SUBDIRS = \
     coretest \
     core \
-    cmake
+    cmake \
+    global
 
 installed_cmake.depends = cmake
 
diff --git a/tests/auto/global/aspects_startup_shutdown/aspects_startup_shutdown.pro b/tests/auto/global/aspects_startup_shutdown/aspects_startup_shutdown.pro
new file mode 100644
index 000000000..0319ae1c3
--- /dev/null
+++ b/tests/auto/global/aspects_startup_shutdown/aspects_startup_shutdown.pro
@@ -0,0 +1,7 @@
+TARGET = tst_aspects_startup_shutdown
+CONFIG += testcase
+TEMPLATE = app
+
+SOURCES += tst_aspects_startup_shutdown.cpp
+
+QT += testlib 3dcore 3drender 3dinput 3dextras
diff --git a/tests/auto/global/aspects_startup_shutdown/tst_aspects_startup_shutdown.cpp b/tests/auto/global/aspects_startup_shutdown/tst_aspects_startup_shutdown.cpp
new file mode 100644
index 000000000..9eec010be
--- /dev/null
+++ b/tests/auto/global/aspects_startup_shutdown/tst_aspects_startup_shutdown.cpp
@@ -0,0 +1,210 @@
+/****************************************************************************
+**
+** Copyright (C) 2018 Klaralvdalens Datakonsult AB (KDAB).
+** Contact: https://www.qt.io/licensing/
+**
+** This file is part of the Qt3D module of the Qt Toolkit.
+**
+** $QT_BEGIN_LICENSE:BSD$
+** 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.
+**
+** BSD License Usage
+** Alternatively, you may use this file under the terms of the BSD license
+** as follows:
+**
+** "Redistribution and use in source and binary forms, with or without
+** modification, are permitted provided that the following conditions are
+** met:
+**   * Redistributions of source code must retain the above copyright
+**     notice, this list of conditions and the following disclaimer.
+**   * Redistributions in binary form must reproduce the above copyright
+**     notice, this list of conditions and the following disclaimer in
+**     the documentation and/or other materials provided with the
+**     distribution.
+**   * Neither the name of The Qt Company Ltd nor the names of its
+**     contributors may be used to endorse or promote products derived
+**     from this software without specific prior written permission.
+**
+**
+** THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+** "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+** LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+** A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+** OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+** SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+** LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+** DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+** THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+** (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+** OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE."
+**
+** $QT_END_LICENSE$
+**
+****************************************************************************/
+
+#include <QtTest/QtTest>
+
+#include <Qt3DCore/QEntity>
+#include <Qt3DCore/QTransform>
+#include <Qt3DCore/QAspectEngine>
+
+#include <Qt3DInput/QInputAspect>
+#include <Qt3DInput/QInputSettings>
+
+#include <Qt3DLogic/QLogicAspect>
+
+#include <Qt3DRender/QCamera>
+#include <Qt3DRender/QCameraLens>
+#include <Qt3DRender/QRenderAspect>
+#include <Qt3DRender/QRenderSettings>
+
+#include <Qt3DExtras/QForwardRenderer>
+#include <Qt3DExtras/Qt3DWindow>
+#include <Qt3DExtras/QPhongMaterial>
+#include <Qt3DExtras/QSphereMesh>
+#include <Qt3DExtras/QTorusMesh>
+
+#include <QPropertyAnimation>
+
+namespace {
+
+Qt3DCore::QEntity *createScene(QWindow *w)
+{
+    // Root entity
+    Qt3DCore::QEntity *rootEntity = new Qt3DCore::QEntity;
+
+    // Camera
+    Qt3DRender::QCamera *camera = new Qt3DRender::QCamera(rootEntity);
+    camera->lens()->setPerspectiveProjection(45.0f, 16.0f/9.0f, 0.1f, 1000.0f);
+    camera->setPosition(QVector3D(0, 0, 40.0f));
+    camera->setViewCenter(QVector3D(0, 0, 0));
+
+    // FrameGraph
+    Qt3DRender::QRenderSettings *renderSettings = new Qt3DRender::QRenderSettings();
+    Qt3DExtras::QForwardRenderer *forwardRenderer = new Qt3DExtras::QForwardRenderer();
+    forwardRenderer->setSurface(w);
+    forwardRenderer->setCamera(camera);
+    forwardRenderer->setClearColor(QColor(Qt::blue));
+    renderSettings->setActiveFrameGraph(forwardRenderer);
+    rootEntity->addComponent(renderSettings);
+
+    // InputSettings
+    Qt3DInput::QInputSettings *inputSettigns = new Qt3DInput::QInputSettings();
+    rootEntity->addComponent(inputSettigns);
+
+    // Material
+    Qt3DRender::QMaterial *material = new Qt3DExtras::QPhongMaterial(rootEntity);
+
+    // Torus
+    Qt3DCore::QEntity *torusEntity = new Qt3DCore::QEntity(rootEntity);
+    Qt3DExtras::QTorusMesh *torusMesh = new Qt3DExtras::QTorusMesh;
+    torusMesh->setRadius(5);
+    torusMesh->setMinorRadius(1);
+    torusMesh->setRings(100);
+    torusMesh->setSlices(20);
+
+    Qt3DCore::QTransform *torusTransform = new Qt3DCore::QTransform;
+    torusTransform->setScale3D(QVector3D(1.5, 1, 0.5));
+    torusTransform->setRotation(QQuaternion::fromAxisAndAngle(QVector3D(1, 0, 0), 45.0f));
+
+    torusEntity->addComponent(torusMesh);
+    torusEntity->addComponent(torusTransform);
+    torusEntity->addComponent(material);
+
+    // Sphere
+    Qt3DCore::QEntity *sphereEntity = new Qt3DCore::QEntity(rootEntity);
+    Qt3DExtras::QSphereMesh *sphereMesh = new Qt3DExtras::QSphereMesh;
+    sphereMesh->setRadius(3);
+
+    Qt3DCore::QTransform *sphereTransform = new Qt3DCore::QTransform;
+
+    sphereEntity->addComponent(sphereMesh);
+    sphereEntity->addComponent(sphereTransform);
+    sphereEntity->addComponent(material);
+
+    return rootEntity;
+}
+
+} // anonymous
+
+class tst_AspectsStartupShutdown : public QObject
+{
+    Q_OBJECT
+
+public:
+
+private slots:
+
+    void checkStartupAndShutdownImmediately()
+    {
+        // GIVEN
+        QWindow *win = new QWindow();
+        win->setSurfaceType(QSurface::OpenGLSurface);
+        win->resize(1024, 768);
+        win->show();
+
+        // WHEN
+        Qt3DCore::QAspectEngine *engine = new Qt3DCore::QAspectEngine();
+        engine->registerAspect(new Qt3DRender::QRenderAspect());
+        engine->registerAspect(new Qt3DInput::QInputAspect());
+        engine->registerAspect(new Qt3DLogic::QLogicAspect());
+        QPointer<Qt3DCore::QEntity> scene = createScene(win);
+        engine->setRootEntity(Qt3DCore::QEntityPtr(scene.data()));
+
+        // THEN
+        QCOMPARE(engine->rootEntity().data(), scene.data());
+
+        // WHEN
+        win->close();
+        delete engine;
+
+        // THEN -> shouldn't crash or deadlock
+        delete win;
+    }
+
+    void checkStartupAndShutdownAfterAFewFrames()
+    {
+        // GIVEN
+        QWindow *win = new QWindow();
+        win->setSurfaceType(QSurface::OpenGLSurface);
+        win->resize(1024, 768);
+        win->show();
+
+        // WHEN
+        Qt3DCore::QAspectEngine *engine = new Qt3DCore::QAspectEngine();
+        engine->registerAspect(new Qt3DRender::QRenderAspect());
+        engine->registerAspect(new Qt3DInput::QInputAspect());
+        engine->registerAspect(new Qt3DLogic::QLogicAspect());
+        QPointer<Qt3DCore::QEntity> scene = createScene(win);
+        engine->setRootEntity(Qt3DCore::QEntityPtr(scene.data()));
+
+        // THEN
+        QCOMPARE(engine->rootEntity().data(), scene.data());
+
+        // Allow a few frames
+        for (int i = 0; i < 10; ++i) {
+            QCoreApplication::processEvents();
+            QThread::msleep(16);
+        }
+
+        // WHEN
+        // Right now we need to close the window before destroying
+        // the engine to prevent the render thread from trying to render
+        // while things are being destroyed
+        win->close();
+        delete engine;
+
+        // THEN -> shouldn't crash or deadlock
+        delete win;
+    }
+};
+
+QTEST_MAIN(tst_AspectsStartupShutdown)
+
+#include "tst_aspects_startup_shutdown.moc"
diff --git a/tests/auto/global/global.pro b/tests/auto/global/global.pro
new file mode 100644
index 000000000..7fca20c44
--- /dev/null
+++ b/tests/auto/global/global.pro
@@ -0,0 +1,8 @@
+TEMPLATE = subdirs
+
+QT_FOR_CONFIG += 3dcore
+
+qtConfig(qt3d-render):qtConfig(qt3d-input):qtConfig(qt3d-extras):qtConfig(qt3d-render):qtConfig(qt3d-logic) {
+    SUBDIRS += \
+        aspects_startup_shutdown
+}
diff --git a/tests/auto/render/filtercompatibletechniquejob/tst_filtercompatibletechniquejob.cpp b/tests/auto/render/filtercompatibletechniquejob/tst_filtercompatibletechniquejob.cpp
index 626f7811e..7a199e9e6 100644
--- a/tests/auto/render/filtercompatibletechniquejob/tst_filtercompatibletechniquejob.cpp
+++ b/tests/auto/render/filtercompatibletechniquejob/tst_filtercompatibletechniquejob.cpp
@@ -190,6 +190,8 @@ private Q_SLOTS:
         // THEN
         QCOMPARE(backendFilterCompatibleTechniqueJob.manager(), &techniqueManager);
         QCOMPARE(backendFilterCompatibleTechniqueJob.renderer(), &renderer);
+
+        renderer.shutdown();
     }
 
     void checkRunRendererRunning()
diff --git a/tests/auto/render/sendrendercapturejob/tst_sendrendercapturejob.cpp b/tests/auto/render/sendrendercapturejob/tst_sendrendercapturejob.cpp
index 9cd2054d6..53a646ccd 100644
--- a/tests/auto/render/sendrendercapturejob/tst_sendrendercapturejob.cpp
+++ b/tests/auto/render/sendrendercapturejob/tst_sendrendercapturejob.cpp
@@ -76,6 +76,7 @@ private Q_SLOTS:
         QCOMPARE(data.data()->image.format(), QImage::Format_ARGB32);
 
         // renderCapture will be deallocated by the nodeManagers destructor
+        renderer.shutdown();
     }
 };
 
diff --git a/tests/auto/render/textures/tst_textures.cpp b/tests/auto/render/textures/tst_textures.cpp
index f1b72b33b..8bd4b355c 100644
--- a/tests/auto/render/textures/tst_textures.cpp
+++ b/tests/auto/render/textures/tst_textures.cpp
@@ -211,6 +211,8 @@ private Q_SLOTS:
 
         // THEN
         QCOMPARE(mgrs->glTextureManager()->lookupResource(bt1a->peerId()), mgrs->glTextureManager()->lookupResource(bt1b->peerId()));
+
+        renderer.shutdown();
     }
 
     void shouldCreateDifferentGLTexturess()
@@ -264,6 +266,8 @@ private Q_SLOTS:
         QCOMPARE(glTextures[0]->properties(), glTextures[2]->properties());
         QCOMPARE(glTextures[1]->properties(), glTextures[3]->properties());
         QVERIFY(glTextures[0]->properties() != glTextures[1]->properties());
+
+        renderer.shutdown();
     }
 
     void generatorsShouldCreateSameData()
@@ -334,6 +338,8 @@ private Q_SLOTS:
 
         QCOMPARE(texDataMgr->getData(tg1a), texDataMgr->getData(tg1b));
         QVERIFY(texDataMgr->getData(tg1a) != texDataMgr->getData(tg2));
+
+        renderer.shutdown();
     }
 
     void checkTextureImageInitialState()
@@ -538,6 +544,8 @@ private Q_SLOTS:
             renderer.clearDirtyBits(Qt3DRender::Render::AbstractRenderer::AllDirty);
             backendImage.unsetDirty();
         }
+
+        renderer.shutdown();
     }
 
     void checkTextureImageProperlyReleaseGenerator()
@@ -601,6 +609,8 @@ private Q_SLOTS:
         QCOMPARE(texImgDataMgr->pendingGenerators().size(), 0);
         QVERIFY(texImgDataMgr->getData(frontendGenerator).isNull());
         QVERIFY(texImgDataMgr->getData(backendGenerator).isNull());
+
+        renderer.shutdown();
     }
 
     void checkTextureIsMarkedForDeletion()
@@ -632,6 +642,8 @@ private Q_SLOTS:
         QCOMPARE(texMgr->textureIdsToCleanup().size(), 1);
         QCOMPARE(texMgr->textureIdsToCleanup().first(), frontendTexture->id());
         QVERIFY(texMgr->lookupResource(frontendTexture->id()) == nullptr);
+
+        renderer.shutdown();
     }
 
     void checkTextureDestructionReconstructionWithinSameLoop()
@@ -666,6 +678,8 @@ private Q_SLOTS:
         QVERIFY(backendTexture != nullptr);
         QCOMPARE(texMgr->textureIdsToCleanup().size(), 0);
         QCOMPARE(texMgr->lookupResource(frontendTexture->id()), backendTexture);
+
+        renderer.shutdown();
     }
 
     void checkTextureImageDirtinessPropagatesToTextures()
@@ -777,6 +791,8 @@ private Q_SLOTS:
         QVERIFY(backendTexture1->dirtyFlags() == Qt3DRender::Render::Texture::NotDirty);
         QVERIFY(backendTexture2->dirtyFlags() & Qt3DRender::Render::Texture::DirtyImageGenerators);
         QVERIFY(backendTexture3->dirtyFlags() & Qt3DRender::Render::Texture::DirtyImageGenerators);
+
+        renderer.shutdown();
     }
 };