diff options
author | Stephan Bergmann <sbergman@redhat.com> | 2016-12-01 12:35:36 +0100 |
---|---|---|
committer | Stephan Bergmann <sbergman@redhat.com> | 2016-12-01 15:43:49 +0000 |
commit | 5a20ff53d908b90e7c805316c61b00149dab9f58 (patch) | |
tree | 7cb9ee252831d514fc200079f15c6c7fdd0d47ee | |
parent | c3bec2919fda97f37127c3656314a1441f19ce5a (diff) |
Fix race in ThreadTaskTag
Assume T0 calls ThreadPool::pushTask twice, then ThreadPool::waitUntilDone:
T0 calls ThreadTaskTag::onTaskPushed:
mnTasksWorking = 1, maTasksComplete.reset
T1 runs the 1st task to completion and calls ThreadTaskTag::onTaskWorkerDone:
mnTasksWorking = 0; suspended...
T0 calls ThreadTaskTag::onTaskPushed:
mnTaskWorking = 1, maTasksComplete.reset
T1 continues in the call to ThreadTaskTag::onTaskWorkerDone:
..., maTasksComplete.set
T0 calls ThreadTaskTag::waitUntilDone and immediately returns
T2 only now starts to run the 2nd task
Change-Id: Ic29101a4791fca2a1a4d54b559f10ff706e8a20d
(cherry picked from commit d36b3bcb7e08f7f73709b7afe9b8f9ec22a0fcd2)
Reviewed-on: https://gerrit.libreoffice.org/31485
Tested-by: Jenkins <ci@libreoffice.org>
Reviewed-by: Stephan Bergmann <sbergman@redhat.com>
-rw-r--r-- | comphelper/source/misc/threadpool.cxx | 18 |
1 files changed, 10 insertions, 8 deletions
diff --git a/comphelper/source/misc/threadpool.cxx b/comphelper/source/misc/threadpool.cxx index f399274eed24..286fbf697358 100644 --- a/comphelper/source/misc/threadpool.cxx +++ b/comphelper/source/misc/threadpool.cxx @@ -26,8 +26,9 @@ static thread_local bool gbIsWorkerThread; // used to group thread-tasks for waiting in waitTillDone() class COMPHELPER_DLLPUBLIC ThreadTaskTag { - oslInterlockedCount mnTasksWorking; - osl::Condition maTasksComplete; + osl::Mutex mMutex; + std::size_t mnTasksWorking; + osl::Condition maTasksComplete; public: ThreadTaskTag(); @@ -305,17 +306,18 @@ ThreadTaskTag::ThreadTaskTag() : mnTasksWorking(0) void ThreadTaskTag::onTaskPushed() { - oslInterlockedCount n = osl_atomic_increment(&mnTasksWorking); - assert( n < 65536 ); // sanity checking - (void)n; // avoid -Wunused-variable in release build + osl::MutexGuard g(mMutex); + assert( mnTasksWorking < 65535 ); // sanity checking + ++mnTasksWorking; maTasksComplete.reset(); } void ThreadTaskTag::onTaskWorkerDone() { - sal_Int32 nCount = osl_atomic_decrement(&mnTasksWorking); - assert(nCount >= 0); - if (nCount == 0) + osl::MutexGuard g(mMutex); + assert(mnTasksWorking > 0); + --mnTasksWorking; + if (mnTasksWorking == 0) maTasksComplete.set(); } |