Always lock the work queue.

The worker threads can be running and have not yet responded
to the last _cond.wait() by the time we start compressing again
- at least under valgrind:

loolforkit-nocaps: ./common/RenderTiles.hpp:304: void ThreadPool::run(): Assertion `_working == 0' failed.
==240379== Process terminating with default action of signal 6 (SIGABRT): dumping core
==240379==    at 0x5505322: raise (raise.c:50)
==240379==    by 0x54EE863: abort (abort.c:79)
==240379==    by 0x54EE748: __assert_fail_base.cold (assert.c:92)
==240379==    by 0x54FD9D5: __assert_fail (assert.c:101)
==240379==    by 0x5886BB: ThreadPool::run() (RenderTiles.hpp:304)
==240379==    by 0x56DC02: RenderTiles::doRender(std::shared_ptr<lok::Document>, TileCombined&, PngCache&, ThreadPool&, bool, std::function<void (unsigned char*, int, int, unsigned long, unsigned long, int, int, LibreOfficeKitTileMode)> const&, std::function<void (char const*, unsigned long)> const&) (RenderTiles.hpp:711)
==240379==    by 0x5A0104: Document::renderTiles(TileCombined&, bool) (Kit.cpp:762)
==240379==    by 0x59CF1A: Document::renderCombinedTiles(StringVector const&) (Kit.cpp:719)
==240379==    by 0x59AEC8: Document::drainQueue(std::chrono::time_point<std::chrono::_V2::steady_clock, std::chrono::duration<long, std::ratio<1l, 1000000000l> > > const&) (Kit.cpp:1570)

That means our queue starts processing work as we do the unlocked
queue push - causing some potential badness.

Change-Id: Ib0578dac009376c0676da73a8c1d8960304dc072
Signed-off-by: Michael Meeks <michael.meeks@collabora.com>

1 files changed, 9 insertions, 3 deletions

diff --git a/common/RenderTiles.hpp b/common/RenderTiles.hpp
index d0e002760e..6fae34e0a9 100644
--- a/common/RenderTiles.hpp
+++ b/common/RenderTiles.hpp
@@ -289,8 +289,10 @@ public:
         return _work.size();
     }
 
-    void pushWorkUnlocked(const ThreadFn &fn)
+    void pushWork(const ThreadFn &fn)
     {
+        std::unique_lock< std::mutex > lock(_mutex);
+        assert(_working == 0);
         _work.push(fn);
     }
 
@@ -303,7 +305,11 @@ public:
         _working++;
         lock.unlock();
 
-        fn();
+        try {
+            fn();
+        } catch(...) {
+            LOG_ERR("Exception in thread pool execution.");
+        }
 
         lock.lock();
         _working--;
@@ -700,7 +706,7 @@ namespace RenderTiles
                 renderingIds.push_back(wireId);
 
                 // Queue to be executed later in parallel inside 'run'
-                pngPool.pushWorkUnlocked([=,&output,&pixmap,&tiles,&renderedTiles,&pngCache,&pngMutex](){
+                pngPool.pushWork([=,&output,&pixmap,&tiles,&renderedTiles,&pngCache,&pngMutex](){
 
                         PngCache::CacheData data(new std::vector< char >() );
                         data->reserve(pixmapWidth * pixmapHeight * 1);