Merge branch 'fix_work_queue' into 'master'

Fix hang on exit (#6145)

Closes #6145

See merge request OpenMW/openmw!1004

apps/openmw/engine.cpp

@@ -427,7 +427,8 @@ OMW::Engine::Engine(Files::ConfigurationManager& configurationManager)
 
 OMW::Engine::~Engine()
 {
-    mWorkQueue->stop();
+    if (mScreenCaptureOperation != nullptr)
+        mScreenCaptureOperation->stop();
 
     mEnvironment.cleanup();
 

apps/openmw/engine.hpp

@@ -21,6 +21,7 @@ namespace Resource
 namespace SceneUtil
 {
     class WorkQueue;
+    class AsyncScreenCaptureOperation;
 }
 
 namespace VFS
@@ -67,7 +68,7 @@ namespace OMW
             boost::filesystem::path mResDir;
             osg::ref_ptr<osgViewer::Viewer> mViewer;
             osg::ref_ptr<osgViewer::ScreenCaptureHandler> mScreenCaptureHandler;
-            osg::ref_ptr<osgViewer::ScreenCaptureHandler::CaptureOperation> mScreenCaptureOperation;
+            osg::ref_ptr<SceneUtil::AsyncScreenCaptureOperation> mScreenCaptureOperation;
             std::string mCellName;
             std::vector<std::string> mContentFiles;
             std::vector<std::string> mGroundcoverFiles;

apps/openmw/mwrender/renderingmanager.cpp

@@ -492,7 +492,7 @@ namespace MWRender
 
         workItem->mTextures.emplace_back("textures/_land_default.dds");
 
-        mWorkQueue->addWorkItem(workItem);
+        mWorkQueue->addWorkItem(std::move(workItem));
     }
 
     double RenderingManager::getReferenceTime() const

apps/openmw/mwworld/cellpreloader.cpp

@@ -317,7 +317,7 @@ namespace MWWorld
             if (found->second.mWorkItem)
             {
                 found->second.mWorkItem->abort();
-                mUnrefQueue->push(mPreloadCells[cell].mWorkItem);
+                mUnrefQueue->push(std::move(found->second.mWorkItem));
             }
 
             mPreloadCells.erase(found);

apps/openmw/mwworld/scene.cpp

@@ -3,6 +3,7 @@
 #include <limits>
 #include <chrono>
 #include <thread>
+#include <atomic>
 
 #include <BulletCollision/CollisionDispatch/btCollisionObject.h>
 #include <BulletCollision/CollisionShapes/btCompoundShape.h>
@@ -867,6 +868,11 @@ namespace MWWorld
 
     Scene::~Scene()
     {
+        for (const osg::ref_ptr<SceneUtil::WorkItem>& v : mWorkItems)
+            v->abort();
+
+        for (const osg::ref_ptr<SceneUtil::WorkItem>& v : mWorkItems)
+            v->waitTillDone();
     }
 
     bool Scene::hasCellChanged() const
@@ -1061,6 +1067,9 @@ namespace MWWorld
 
         void doWork() override
         {
+            if (mAborted)
+                return;
+
             try
             {
                 mSceneManager->getTemplate(mMesh);
@@ -1069,9 +1078,16 @@ namespace MWWorld
             {
             }
         }
+
+        void abort() override
+        {
+            mAborted = true;
+        }
+
     private:
         std::string mMesh;
         Resource::SceneManager* mSceneManager;
+        std::atomic_bool mAborted {false};
     };
 
     void Scene::preload(const std::string &mesh, bool useAnim)
@@ -1081,7 +1097,13 @@ namespace MWWorld
             mesh_ = Misc::ResourceHelpers::correctActorModelPath(mesh_, mRendering.getResourceSystem()->getVFS());
 
         if (!mRendering.getResourceSystem()->getSceneManager()->checkLoaded(mesh_, mRendering.getReferenceTime()))
-            mRendering.getWorkQueue()->addWorkItem(new PreloadMeshItem(mesh_, mRendering.getResourceSystem()->getSceneManager()));
+        {
+            osg::ref_ptr<PreloadMeshItem> item(new PreloadMeshItem(mesh_, mRendering.getResourceSystem()->getSceneManager()));
+            mRendering.getWorkQueue()->addWorkItem(item);
+            const auto isDone = [] (const osg::ref_ptr<SceneUtil::WorkItem>& v) { return v->isDone(); };
+            mWorkItems.erase(std::remove_if(mWorkItems.begin(), mWorkItems.end(), isDone), mWorkItems.end());
+            mWorkItems.emplace_back(std::move(item));
+        }
     }
 
     void Scene::preloadCells(float dt)

apps/openmw/mwworld/scene.hpp

@@ -3,6 +3,7 @@
 
 #include <osg/Vec4i>
 #include <osg/Vec2i>
+#include <osg/ref_ptr>
 
 #include "ptr.hpp"
 #include "globals.hpp"
@@ -10,6 +11,7 @@
 #include <set>
 #include <memory>
 #include <unordered_map>
+#include <vector>
 
 #include <components/misc/constants.hpp>
 
@@ -49,6 +51,11 @@ namespace MWPhysics
     class PhysicsSystem;
 }
 
+namespace SceneUtil
+{
+    class WorkItem;
+}
+
 namespace MWWorld
 {
     class Player;
@@ -91,6 +98,8 @@ namespace MWWorld
 
             std::set<ESM::RefNum> mPagedRefs;
 
+            std::vector<osg::ref_ptr<SceneUtil::WorkItem>> mWorkItems;
+
             void insertCell (CellStore &cell, Loading::Listener* loadingListener, bool onlyObjects, bool test = false);
             osg::Vec2i mCurrentGridCenter;
 

components/sceneutil/screencapture.cpp

@@ -15,6 +15,7 @@
 #include <iomanip>
 #include <sstream>
 #include <string>
+#include <atomic>
 
 namespace
 {
@@ -32,6 +33,9 @@ namespace
 
             void doWork() override
             {
+                if (mAborted)
+                    return;
+
                 try
                 {
                     (*mImpl)(*mImage, mContextId);
@@ -42,10 +46,16 @@ namespace
                 }
             }
 
+            void abort() override
+            {
+                mAborted = true;
+            }
+
         private:
             const osg::ref_ptr<osgViewer::ScreenCaptureHandler::CaptureOperation> mImpl;
             const osg::ref_ptr<const osg::Image> mImage;
             const unsigned int mContextId;
+            std::atomic_bool mAborted {false};
     };
 }
 
@@ -130,8 +140,27 @@ namespace SceneUtil
         assert(mImpl != nullptr);
     }
 
+    AsyncScreenCaptureOperation::~AsyncScreenCaptureOperation()
+    {
+        stop();
+    }
+
+    void AsyncScreenCaptureOperation::stop()
+    {
+        for (const osg::ref_ptr<SceneUtil::WorkItem>& item : *mWorkItems.lockConst())
+            item->abort();
+
+        for (const osg::ref_ptr<SceneUtil::WorkItem>& item : *mWorkItems.lockConst())
+            item->waitTillDone();
+    }
+
     void AsyncScreenCaptureOperation::operator()(const osg::Image& image, const unsigned int context_id)
     {
-        mQueue->addWorkItem(new ScreenCaptureWorkItem(mImpl, image, context_id));
+        osg::ref_ptr<SceneUtil::WorkItem> item(new ScreenCaptureWorkItem(mImpl, image, context_id));
+        mQueue->addWorkItem(item);
+        const auto isDone = [] (const osg::ref_ptr<SceneUtil::WorkItem>& v) { return v->isDone(); };
+        const auto workItems = mWorkItems.lock();
+        workItems->erase(std::remove_if(workItems->begin(), workItems->end(), isDone), workItems->end());
+        workItems->emplace_back(std::move(item));
     }
 }

components/sceneutil/screencapture.hpp

@@ -1,10 +1,13 @@
 #ifndef OPENMW_COMPONENTS_SCENEUTIL_SCREENCAPTURE_H
 #define OPENMW_COMPONENTS_SCENEUTIL_SCREENCAPTURE_H
 
+#include <components/misc/guarded.hpp>
+
 #include <osg/ref_ptr>
 #include <osgViewer/ViewerEventHandlers>
 
 #include <string>
+#include <vector>
 
 namespace osg
 {
@@ -14,6 +17,7 @@ namespace osg
 namespace SceneUtil
 {
     class WorkQueue;
+    class WorkItem;
 
     std::string writeScreenshotToFile(const std::string& screenshotPath, const std::string& screenshotFormat,
                                       const osg::Image& image);
@@ -38,11 +42,16 @@ namespace SceneUtil
             AsyncScreenCaptureOperation(osg::ref_ptr<SceneUtil::WorkQueue> queue,
                                         osg::ref_ptr<osgViewer::ScreenCaptureHandler::CaptureOperation> impl);
 
+            ~AsyncScreenCaptureOperation();
+
+            void stop();
+
             void operator()(const osg::Image& image, const unsigned int context_id) override;
 
         private:
             const osg::ref_ptr<SceneUtil::WorkQueue> mQueue;
             const osg::ref_ptr<osgViewer::ScreenCaptureHandler::CaptureOperation> mImpl;
+            Misc::ScopeGuarded<std::vector<osg::ref_ptr<SceneUtil::WorkItem>>> mWorkItems;
     };
 }
 

components/sceneutil/workqueue.cpp

@@ -74,9 +74,9 @@ void WorkQueue::addWorkItem(osg::ref_ptr<WorkItem> item, bool front)
 
     std::unique_lock<std::mutex> lock(mMutex);
     if (front)
-        mQueue.push_front(item);
+        mQueue.push_front(std::move(item));
     else
-        mQueue.push_back(item);
+        mQueue.push_back(std::move(item));
     mCondition.notify_one();
 }
 
@@ -89,12 +89,11 @@ osg::ref_ptr<WorkItem> WorkQueue::removeWorkItem()
     }
     if (!mQueue.empty())
     {
-        osg::ref_ptr<WorkItem> item = mQueue.front();
+        osg::ref_ptr<WorkItem> item = std::move(mQueue.front());
         mQueue.pop_front();
         return item;
     }
-    else
+    return nullptr;
-        return nullptr;
 }
 
 unsigned int WorkQueue::getNumItems() const