comphelper:: fix MSVC hang in ThreadPool::shutdown(), try #2

This takes a different approach than commit
9899ffd244dd367ba69dffe1f21f4f0222064a46.

Change the ThreadPool to automatically shutdown and join all threads
whenever waitUntilDone() is called.  Then start the threads again
in pushTask().

Because the ThreadPool is meant to be used synchronously with
waitUntilDone() called after adding all required tasks, this should
obviate the need to call shutdown() before process exit, as
there won't be any threads running at that point.

Change-Id: I2b8e639004a94cf05ccb4522aa1f0d3dac88a936
Reviewed-on: https://gerrit.libreoffice.org/35510
Reviewed-by: Michael Meeks <michael.meeks@collabora.com>
Tested-by: Jenkins <ci@libreoffice.org>

1 files changed, 29 insertions, 10 deletions

diff --git a/comphelper/source/misc/threadpool.cxx b/comphelper/source/misc/threadpool.cxx
index 712009d8a2b4..3aaf344c3ba8 100644
--- a/comphelper/source/misc/threadpool.cxx
+++ b/comphelper/source/misc/threadpool.cxx
@@ -75,21 +75,20 @@ public:
     }
 };
 
-ThreadPool::ThreadPool( sal_Int32 nWorkers ) :
-    mbTerminate( false )
+ThreadPool::ThreadPool(sal_Int32 nWorkers)
+    : mbTerminate(true)
+    , mnWorkers(nWorkers)
 {
-    std::unique_lock< std::mutex > aGuard( maMutex );
-
-    for( sal_Int32 i = 0; i < nWorkers; i++ )
-        maWorkers.push_back( new ThreadWorker( this ) );
-
-    for(rtl::Reference<ThreadWorker> & rpWorker : maWorkers)
-        rpWorker->launch();
 }
 
 ThreadPool::~ThreadPool()
 {
-    shutdown();
+    // note: calling shutdown from global variable dtor blocks forever on Win7
+    // note2: there isn't enough MSVCRT left on exit to call assert() properly
+    // so these asserts just print something to stderr but exit status is
+    // still 0, but hopefully they will be more helpful on non-WNT platforms
+    assert(mbTerminate);
+    assert(maTasks.empty());
 }
 
 struct ThreadPoolStatic : public rtl::StaticWithInit< std::shared_ptr< ThreadPool >,
@@ -136,7 +135,11 @@ void ThreadPool::shutdown()
         return;
 
     std::unique_lock< std::mutex > aGuard( maMutex );
+    shutdownLocked(aGuard);
+}
 
+void ThreadPool::shutdownLocked(std::unique_lock<std::mutex>& aGuard)
+{
     if( maWorkers.empty() )
     { // no threads at all -> execute the work in-line
         ThreadTask *pTask;
@@ -173,6 +176,14 @@ void ThreadPool::pushTask( ThreadTask *pTask )
 {
     std::unique_lock< std::mutex > aGuard( maMutex );
 
+    mbTerminate = false;
+
+    if (maWorkers.size() < mnWorkers && maWorkers.size() <= maTasks.size())
+    {
+        maWorkers.push_back( new ThreadWorker( this ) );
+        maWorkers.back()->launch();
+    }
+
     pTask->mpTag->onTaskPushed();
     maTasks.insert( maTasks.begin(), pTask );
 
@@ -217,6 +228,14 @@ void ThreadPool::waitUntilDone(const std::shared_ptr<ThreadTaskTag>& rTag)
     }
 
     rTag->waitUntilDone();
+
+    {
+        std::unique_lock< std::mutex > aGuard( maMutex );
+        if (maTasks.empty()) // check if there are still tasks from another tag
+        {
+            shutdownLocked(aGuard);
+        }
+    }
 }
 
 std::shared_ptr<ThreadTaskTag> ThreadPool::createThreadTaskTag()