Make ThreadPool::pushTask take param by std::unique_ptr

And fix leak in XclExpRowBuffer::Finalize, was not freeing the
synchronous task it creates

Change-Id: Id1e9ddb5d968e6b95d9d2b5ca0c9e50774580182
Reviewed-on: https://gerrit.libreoffice.org/56874
Tested-by: Jenkins
Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>

14 files changed, 39 insertions, 35 deletions

diff --git a/comphelper/source/misc/threadpool.cxx b/comphelper/source/misc/threadpool.cxx
index 23e363213206..a910ef08e138 100644
--- a/comphelper/source/misc/threadpool.cxx
+++ b/comphelper/source/misc/threadpool.cxx
@@ -62,12 +62,13 @@ public:
 
         while( !mpPool->mbTerminate )
         {
-            ThreadTask *pTask = mpPool->popWorkLocked( aGuard, true );
+            std::unique_ptr<ThreadTask> pTask = mpPool->popWorkLocked( aGuard, true );
             if( pTask )
             {
                 aGuard.unlock();
 
-                pTask->execAndDelete();
+                pTask->exec();
+                pTask.reset();
 
                 aGuard.lock();
             }
@@ -142,9 +143,9 @@ void ThreadPool::shutdownLocked(std::unique_lock<std::mutex>& aGuard)
 {
     if( maWorkers.empty() )
     { // no threads at all -> execute the work in-line
-        ThreadTask *pTask;
+        std::unique_ptr<ThreadTask> pTask;
         while ( ( pTask = popWorkLocked(aGuard, false) ) )
-            pTask->execAndDelete();
+            pTask->exec();
     }
     else
     {
@@ -175,7 +176,7 @@ void ThreadPool::shutdownLocked(std::unique_lock<std::mutex>& aGuard)
     }
 }
 
-void ThreadPool::pushTask( ThreadTask *pTask )
+void ThreadPool::pushTask( std::unique_ptr<ThreadTask> pTask )
 {
     std::unique_lock< std::mutex > aGuard( maMutex );
 
@@ -188,18 +189,18 @@ void ThreadPool::pushTask( ThreadTask *pTask )
     }
 
     pTask->mpTag->onTaskPushed();
-    maTasks.insert( maTasks.begin(), pTask );
+    maTasks.insert( maTasks.begin(), std::move(pTask) );
 
     maTasksChanged.notify_one();
 }
 
-ThreadTask *ThreadPool::popWorkLocked( std::unique_lock< std::mutex > & rGuard, bool bWait )
+std::unique_ptr<ThreadTask> ThreadPool::popWorkLocked( std::unique_lock< std::mutex > & rGuard, bool bWait )
 {
     do
     {
         if( !maTasks.empty() )
         {
-            ThreadTask *pTask = maTasks.back();
+            std::unique_ptr<ThreadTask> pTask = std::move(maTasks.back());
             maTasks.pop_back();
             return pTask;
         }
@@ -223,10 +224,10 @@ void ThreadPool::waitUntilDone(const std::shared_ptr<ThreadTaskTag>& rTag)
 
         if( maWorkers.empty() )
         { // no threads at all -> execute the work in-line
-            ThreadTask *pTask;
+            std::unique_ptr<ThreadTask> pTask;
             while (!rTag->isDone() &&
                    ( pTask = popWorkLocked(aGuard, false) ) )
-                pTask->execAndDelete();
+                pTask->exec();
         }
     }
 
@@ -256,7 +257,7 @@ ThreadTask::ThreadTask(const std::shared_ptr<ThreadTaskTag>& pTag)
 {
 }
 
-void ThreadTask::execAndDelete()
+void ThreadTask::exec()
 {
     std::shared_ptr<ThreadTaskTag> pTag(mpTag);
     try {
@@ -271,7 +272,6 @@ void ThreadTask::execAndDelete()
         SAL_WARN("comphelper", "exception in thread worker while calling doWork(): " << e);
     }
 
-    delete this;
     pTag->onTaskWorkerDone();
 }
 
diff --git a/drawinglayer/source/primitive2d/sceneprimitive2d.cxx b/drawinglayer/source/primitive2d/sceneprimitive2d.cxx
index 35024d9bc684..0249bd7a48ac 100644
--- a/drawinglayer/source/primitive2d/sceneprimitive2d.cxx
+++ b/drawinglayer/source/primitive2d/sceneprimitive2d.cxx
@@ -424,8 +424,8 @@ namespace drawinglayer
                                 aBZPixelRaster,
                                 nLinesPerThread * a,
                                 a + 1 == nThreadCount ? aBZPixelRaster.getHeight() : nLinesPerThread * (a + 1)));
-                            Executor* pExecutor = new Executor(aTag, std::move(pNewZBufferProcessor3D), getChildren3D());
-                            rThreadPool.pushTask(pExecutor);
+                            std::unique_ptr<Executor> pExecutor(new Executor(aTag, std::move(pNewZBufferProcessor3D), getChildren3D()));
+                            rThreadPool.pushTask(std::move(pExecutor));
                         }
 
                         rThreadPool.waitUntilDone(aTag);
diff --git a/include/comphelper/threadpool.hxx b/include/comphelper/threadpool.hxx
index 66ade62f3f8f..ddca4d849245 100644
--- a/include/comphelper/threadpool.hxx
+++ b/include/comphelper/threadpool.hxx
@@ -27,10 +27,11 @@ class ThreadPool;
 class COMPHELPER_DLLPUBLIC ThreadTask
 {
 friend class ThreadPool;
+friend struct std::default_delete<ThreadTask>;
     std::shared_ptr<ThreadTaskTag>  mpTag;
 
-    /// execute and delete this task
-    void      execAndDelete();
+    /// execute this task
+    void      exec();
 protected:
     /// override to get your task performed by the pool
     virtual void doWork() = 0;
@@ -62,7 +63,7 @@ public:
     ~ThreadPool();
 
     /// push a new task onto the work queue
-    void        pushTask( ThreadTask *pTask /* takes ownership */ );
+    void        pushTask( std::unique_ptr<ThreadTask> pTask);
 
     /// wait until all queued tasks associated with the tag are completed
     void        waitUntilDone(const std::shared_ptr<ThreadTaskTag>&);
@@ -84,7 +85,7 @@ private:
         @param  bWait - if set wait until task present or termination
         @return a new task to perform, or NULL if list empty or terminated
     */
-    ThreadTask *popWorkLocked( std::unique_lock< std::mutex > & rGuard, bool bWait );
+    std::unique_ptr<ThreadTask> popWorkLocked( std::unique_lock< std::mutex > & rGuard, bool bWait );
     void shutdownLocked(std::unique_lock<std::mutex>&);
 
     /// signalled when all in-progress tasks are complete
@@ -92,7 +93,7 @@ private:
     std::condition_variable maTasksChanged;
     bool                    mbTerminate;
     std::size_t             mnWorkers;
-    std::vector< ThreadTask * >   maTasks;
+    std::vector< std::unique_ptr<ThreadTask> >   maTasks;
     std::vector< rtl::Reference< ThreadWorker > > maWorkers;
 };
 
diff --git a/package/inc/ZipOutputStream.hxx b/package/inc/ZipOutputStream.hxx
index a0a8f3ad24ac..2c3e26fab4ac 100644
--- a/package/inc/ZipOutputStream.hxx
+++ b/package/inc/ZipOutputStream.hxx
@@ -47,7 +47,7 @@ public:
         const css::uno::Reference< css::io::XOutputStream > &xOStream );
     ~ZipOutputStream();
 
-    void addDeflatingThread( ZipOutputEntry *pEntry, comphelper::ThreadTask *pThreadTask );
+    void addDeflatingThread( ZipOutputEntry *pEntry, std::unique_ptr<comphelper::ThreadTask> pThreadTask );
 
     /// @throws css::io::IOException
     /// @throws css::uno::RuntimeException
diff --git a/package/qa/cppunit/test_package.cxx b/package/qa/cppunit/test_package.cxx
index 04e6b0643e96..0c49e55dca11 100644
--- a/package/qa/cppunit/test_package.cxx
+++ b/package/qa/cppunit/test_package.cxx
@@ -13,6 +13,7 @@
 #include <comphelper/threadpool.hxx>
 #include <com/sun/star/packages/zip/ZipFileAccess.hpp>
 #include <com/sun/star/beans/NamedValue.hpp>
+#include <o3tl/make_unique.hxx>
 
 #include <iterator>
 
@@ -159,7 +160,7 @@ namespace
                 mxNA->getByName(aName) >>= xStrm;
 
                 CPPUNIT_ASSERT(xStrm.is());
-                aPool.pushTask(new Worker(pTag, xStrm, *itBuf));
+                aPool.pushTask(o3tl::make_unique<Worker>(pTag, xStrm, *itBuf));
             }
 
             aPool.waitUntilDone(pTag);
diff --git a/package/source/zipapi/ZipOutputStream.cxx b/package/source/zipapi/ZipOutputStream.cxx
index f7f54d6ff421..94f8d024dd7c 100644
--- a/package/source/zipapi/ZipOutputStream.cxx
+++ b/package/source/zipapi/ZipOutputStream.cxx
@@ -68,9 +68,9 @@ void ZipOutputStream::setEntry( ZipEntry *pEntry )
     }
 }
 
-void ZipOutputStream::addDeflatingThread( ZipOutputEntry *pEntry, comphelper::ThreadTask *pThread )
+void ZipOutputStream::addDeflatingThread( ZipOutputEntry *pEntry, std::unique_ptr<comphelper::ThreadTask> pThread )
 {
-    comphelper::ThreadPool::getSharedOptimalPool().pushTask(pThread);
+    comphelper::ThreadPool::getSharedOptimalPool().pushTask(std::move(pThread));
     m_aEntries.push_back(pEntry);
 }
 
diff --git a/package/source/zippackage/ZipPackageStream.cxx b/package/source/zippackage/ZipPackageStream.cxx
index 908a093cb62d..a558be575127 100644
--- a/package/source/zippackage/ZipPackageStream.cxx
+++ b/package/source/zippackage/ZipPackageStream.cxx
@@ -50,6 +50,7 @@
 #include <cppuhelper/exc_hlp.hxx>
 #include <cppuhelper/supportsservice.hxx>
 #include <cppuhelper/typeprovider.hxx>
+#include <o3tl/make_unique.hxx>
 
 #include <rtl/instance.hxx>
 #include <rtl/random.h>
@@ -835,7 +836,7 @@ bool ZipPackageStream::saveChild(
                     // Start a new thread deflating this zip entry
                     ZipOutputEntry *pZipEntry = new ZipOutputEntry(
                             m_xContext, *pTempEntry, this, bToBeEncrypted);
-                    rZipOut.addDeflatingThread( pZipEntry, new DeflateThread(rZipOut.getThreadTaskTag(), pZipEntry, xStream) );
+                    rZipOut.addDeflatingThread( pZipEntry, o3tl::make_unique<DeflateThread>(rZipOut.getThreadTaskTag(), pZipEntry, xStream) );
                 }
                 else
                 {
diff --git a/sc/source/core/data/formulacell.cxx b/sc/source/core/data/formulacell.cxx
index 3800397a9ca7..9d697e2a4e6b 100644
--- a/sc/source/core/data/formulacell.cxx
+++ b/sc/source/core/data/formulacell.cxx
@@ -4616,7 +4616,7 @@ bool ScFormulaCell::InterpretFormulaGroupThreading(sc::FormulaLogger::GroupScope
             for (int i = 0; i < nThreadCount; ++i)
             {
                 contexts[i] = new ScInterpreterContext(*pDocument, pNonThreadedFormatter);
-                rThreadPool.pushTask(new Executor(aTag, i, nThreadCount, pDocument, contexts[i], mxGroup->mpTopCell->aPos, mxGroup->mnLength));
+                rThreadPool.pushTask(o3tl::make_unique<Executor>(aTag, i, nThreadCount, pDocument, contexts[i], mxGroup->mpTopCell->aPos, mxGroup->mnLength));
             }
 
             SAL_INFO("sc.threaded", "Joining threads");
diff --git a/sc/source/core/tool/formulagroup.cxx b/sc/source/core/tool/formulagroup.cxx
index 0da064c19afa..cd4a4ccecaf3 100644
--- a/sc/source/core/tool/formulagroup.cxx
+++ b/sc/source/core/tool/formulagroup.cxx
@@ -400,7 +400,7 @@ bool FormulaGroupInterpreterSoftware::interpret(ScDocument& rDoc, const ScAddres
             if ( nRemaining )
                 --nRemaining;
             SCROW nLast = nStart + nCount - 1;
-            rThreadPool.pushTask(new Executor(aTag, rCode, aTmpPos, rTopPos, rDoc, pFormatter, aResults, nStart, nLast));
+            rThreadPool.pushTask(o3tl::make_unique<Executor>(aTag, rCode, aTmpPos, rTopPos, rDoc, pFormatter, aResults, nStart, nLast));
             aTmpPos.IncRow(nCount);
             nLeft -= nCount;
             nStart = nLast + 1;
diff --git a/sc/source/filter/excel/xetable.cxx b/sc/source/filter/excel/xetable.cxx
index 7ca907dff18f..b68b094d0660 100644
--- a/sc/source/filter/excel/xetable.cxx
+++ b/sc/source/filter/excel/xetable.cxx
@@ -2196,9 +2196,9 @@ void XclExpRowBuffer::Finalize( XclExpDefaultRowData& rDefRowData, const ScfUInt
     {
         comphelper::ThreadPool &rPool = comphelper::ThreadPool::getSharedOptimalPool();
         std::shared_ptr<comphelper::ThreadTaskTag> pTag = comphelper::ThreadPool::createThreadTaskTag();
-        std::vector<RowFinalizeTask*> aTasks(nThreads, nullptr);
+        std::vector<std::unique_ptr<RowFinalizeTask>> aTasks(nThreads);
         for ( size_t i = 0; i < nThreads; i++ )
-            aTasks[ i ] = new RowFinalizeTask( pTag, rColXFIndexes, i == 0 );
+            aTasks[ i ].reset( new RowFinalizeTask( pTag, rColXFIndexes, i == 0 ) );
 
         RowMap::iterator itr, itrBeg = maRowMap.begin(), itrEnd = maRowMap.end();
         size_t nIdx = 0;
@@ -2206,7 +2206,7 @@ void XclExpRowBuffer::Finalize( XclExpDefaultRowData& rDefRowData, const ScfUInt
             aTasks[ nIdx % nThreads ]->push_back( itr->second.get() );
 
         for ( size_t i = 1; i < nThreads; i++ )
-            rPool.pushTask( aTasks[ i ] );
+            rPool.pushTask( std::move(aTasks[ i ]) );
 
         // Progress bar updates must be synchronous to avoid deadlock
         aTasks[0]->doWork();
diff --git a/sc/source/filter/oox/workbookfragment.cxx b/sc/source/filter/oox/workbookfragment.cxx
index 9514a5b3d692..068df46ff443 100644
--- a/sc/source/filter/oox/workbookfragment.cxx
+++ b/sc/source/filter/oox/workbookfragment.cxx
@@ -65,6 +65,7 @@
 #include <salhelper/thread.hxx>
 #include <comphelper/threadpool.hxx>
 #include <osl/conditn.hxx>
+#include <o3tl/make_unique.hxx>
 
 #include <algorithm>
 #include <queue>
@@ -326,7 +327,7 @@ void importSheetFragments( WorkbookFragment& rWorkbookHandler, SheetFragmentVect
          pProgress->setCustomRowProgress(
                      aProgressUpdater.wrapProgress(
                              pProgress->getRowProgress() ) );
-         rSharedPool.pushTask( new WorkerThread( pTag, rWorkbookHandler, it->second,
+         rSharedPool.pushTask( o3tl::make_unique<WorkerThread>( pTag, rWorkbookHandler, it->second,
                                            /* ref */ nSheetsLeft ) );
          nSheetsLeft++;
      }
diff --git a/sw/source/core/ole/ndole.cxx b/sw/source/core/ole/ndole.cxx
index 6101302bf40b..584dcc8e89f0 100644
--- a/sw/source/core/ole/ndole.cxx
+++ b/sw/source/core/ole/ndole.cxx
@@ -1048,8 +1048,8 @@ drawinglayer::primitive2d::Primitive2DContainer const & SwOLEObj::tryToGetChartC
                 if(!m_pDeflateData)
                 {
                     m_pDeflateData.reset( new DeflateData(aXModel) );
-                    DeflateThread* pNew = new DeflateThread(*m_pDeflateData);
-                    comphelper::ThreadPool::getSharedOptimalPool().pushTask(pNew);
+                    std::unique_ptr<DeflateThread> pNew( new DeflateThread(*m_pDeflateData) );
+                    comphelper::ThreadPool::getSharedOptimalPool().pushTask(std::move(pNew));
                 }
             }
         }
diff --git a/vcl/source/bitmap/BitmapScaleSuperFilter.cxx b/vcl/source/bitmap/BitmapScaleSuperFilter.cxx
index f096bd7429ff..01e922a9fa57 100644
--- a/vcl/source/bitmap/BitmapScaleSuperFilter.cxx
+++ b/vcl/source/bitmap/BitmapScaleSuperFilter.cxx
@@ -1038,14 +1038,14 @@ BitmapEx BitmapScaleSuperFilter::execute(BitmapEx const& rBitmap)
                     long nStripY = nStartY;
                     for ( sal_uInt32 t = 0; t < nThreads - 1; t++ )
                     {
-                        ScaleTask *pTask = new ScaleTask( pTag, pScaleRangeFn );
+                        std::unique_ptr<ScaleTask> pTask(new ScaleTask( pTag, pScaleRangeFn ));
                         for ( sal_uInt32 j = 0; j < nStripsPerThread; j++ )
                         {
                             ScaleRangeContext aRC( &aContext, nStripY );
                             pTask->push( aRC );
                             nStripY += SCALE_THREAD_STRIP;
                         }
-                        rShared.pushTask( pTask );
+                        rShared.pushTask( std::move(pTask) );
                     }
                     // finish any remaining bits here
                     pScaleRangeFn( aContext, nStripY, nEndY );
diff --git a/vcl/source/filter/graphicfilter.cxx b/vcl/source/filter/graphicfilter.cxx
index b928b83f7695..fea987a63520 100644
--- a/vcl/source/filter/graphicfilter.cxx
+++ b/vcl/source/filter/graphicfilter.cxx
@@ -1385,7 +1385,7 @@ void GraphicFilter::ImportGraphics(std::vector< std::shared_ptr<Graphic> >& rGra
                         rContext.m_pAccess = o3tl::make_unique<BitmapScopedWriteAccess>(rBitmap);
                         pStream->Seek(rContext.m_nStreamBegin);
                         if (bThreads)
-                            rSharedPool.pushTask(new GraphicImportTask(pTag, rContext));
+                            rSharedPool.pushTask(o3tl::make_unique<GraphicImportTask>(pTag, rContext));
                         else
                             GraphicImportTask::doImport(rContext);
                     }