Commit Graph

28 Commits

Author SHA1 Message Date
Caolán McNamara
7b9f22fb62 try and silence coverity#1403659 Data race condition
Change-Id: Ibe8629b9f61230b41840351a3486050e6ad9eb8c
2017-04-07 09:50:32 +01:00
Michael Stahl
64c09371d6 comphelper::ThreadPool: guard against concurrent shutdown/pushTask
To join a thread, the mutex must be released - another thread
in pushTask() could add a new thread to maWorkers at that point,
which must of course not be deleted.

Avoid the problem by transferring ownership of the to-be-deleted
threads to the calling thread.

(regression from bdaa13a877)

Change-Id: I9d4fcfe4cb46a336586b5663934a12d47b2d8ccb
2017-03-22 11:39:32 +01:00
Michael Stahl
bdaa13a877 comphelper:: fix MSVC hang in ThreadPool::shutdown(), try #2
This takes a different approach than commit
9899ffd244.

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>
2017-03-21 16:15:36 +00:00
Miklos Vajna
3902bb7a45 Revert "comphelper: fix MSVC hang in ThreadPool::shutdown()"
As it causes "unopkg.bin:
/home/tdf/lode/jenkins/workspace/lo_gerrit/Config/linux_clang_dbgutil_64/comphelper/source/misc/threadpool.cxx:96:
comphelper::ThreadPool::~ThreadPool(): Assertion `mbTerminate' failed."
in
<https://ci.libreoffice.org/job/lo_gerrit/8283/Config=linux_clang_dbgutil_64/console>
and also locally. Revert till it's clear if that assert() should be a
SAL_WARN() or unopkg has to be fixed.

This reverts commit 9899ffd244.

Change-Id: I72902f7da410012340aa8231d84c6871a3f7b976
2017-03-13 12:11:04 +01:00
Michael Stahl
9899ffd244 comphelper: fix MSVC hang in ThreadPool::shutdown()
Commit aa68c99d88 added some code using
std::condition_variable to comphelper.

Built with MSVC 2017, this causes many cppunittester.exe processes to
deadlock in ThreadPool::shutdown():

    maTasksChanged.notify_all();

This ultimately calls NtReleaseKeyedEvent(), which never returns.

The reason appears to be a bug in Windows 7, for which a "hotfix"[1] is
avaiable here, but it's apparently not distributed via Windows Update
so we likely can't rely on users or even developers having this installed.

However, the documentation of DllMain[2] and ExitProcess[3] indicates
that during shutdown, by the time global destructors are invoked
all threads other than the one that called ExitProcess have already
been terminated.

Returning from main() implicitly calls ExitProcess [4].

As it turns out the problem only happens for some CppUnitTests because
soffice.bin will call ThreadPool::shutdown() from Desktop::doShutdown()
while it is still safe.

[1] http://support.microsoft.com/kb/2582203
[2] https://msdn.microsoft.com/en-US/library/windows/desktop/ms682583(v=vs.85).aspx
[3] https://msdn.microsoft.com/en-us/library/windows/desktop/ms682658(v=vs.85).aspx
[4] https://blogs.msdn.microsoft.com/oldnewthing/20100827-00/?p=13023

Change-Id: I6137461ca7efe9a5fbe4f8f8478fb96de3570469
Reviewed-on: https://gerrit.libreoffice.org/35066
Tested-by: Jenkins <ci@libreoffice.org>
Reviewed-by: Michael Stahl <mstahl@redhat.com>
2017-03-11 10:08:06 +00:00
Noel Grandin
46d3163f77 loplugin:unusedmethods
Change-Id: Ife4c8d948ffa116f044d43903de9485e43cfcae5
Reviewed-on: https://gerrit.libreoffice.org/32336
Tested-by: Jenkins <ci@libreoffice.org>
Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>
2016-12-23 04:46:35 +00:00
Stephan Bergmann
21f6405cbe This is presumably not meant to be exported
Change-Id: Id11393c44ddd217d3def259ea5a8df9f8bb27711
2016-12-07 22:17:56 +01:00
Michael Meeks
aa68c99d88 tdf#104126 - comphelper thread-pool, use reliable std::condition_variable.
The existing osl::Condition is an API and reliability disaster area.

Change-Id: I3be84e1c6a83e58c43c40c9c8720790d923a6694
Reviewed-on: https://gerrit.libreoffice.org/31163
Tested-by: Jenkins <ci@libreoffice.org>
Reviewed-by: Michael Meeks <michael.meeks@collabora.com>
Tested-by: Michael Meeks <michael.meeks@collabora.com>
2016-12-01 18:44:08 +00:00
Stephan Bergmann
d36b3bcb7e 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
2016-12-01 12:35:36 +01:00
Stephan Bergmann
f8e3e523e2 Rewrite some (trivial) assignments inside if/while conditions: comphelper
Change-Id: I8361f62199f45dbced45e5d4a4d5eeddf1c42d67
2016-11-29 17:21:47 +01:00
lbenes
a4a71214d6 comphelper: fix MSVC 2015 build by removing pointless catch
In C++11 the destructors are implicitly noexcept.

Change-Id: I37e78e39bcc19dfbc81a781a5b353e49f09ae942
Reviewed-on: https://gerrit.libreoffice.org/27708
Tested-by: Jenkins <ci@libreoffice.org>
Reviewed-by: Noel Grandin <noelgrandin@gmail.com>
2016-08-01 06:30:12 +00:00
Caolán McNamara
5733910b2b change from a 2 min dbgutil failure to a 3 min
to try and align with the crashtesting timeout

Change-Id: Ife3a4a3d63bbb9d9d5b612675e3728900262bf6c
2016-07-27 11:02:28 +01:00
Caolán McNamara
54d8d7718b fix build
Change-Id: I90d6826e4544fd39120982f80d41e237a5edbab6
2016-07-18 09:01:39 +01:00
Noel Grandin
147d18ad4a -Werror=enum-compare
Change-Id: I6bb2c9bcefd9dbb0efd262b1462625a157d11e6f
2016-07-18 09:53:44 +02:00
Noel Grandin
09b0ade4f8 remove some now unnecessary debug trace
Change-Id: I15fef941c5a9c9d7627ca22029a95c8e6928ee20
2016-07-18 09:21:30 +02:00
Noel Grandin
76ad32bec8 add tagging to ThreadTasks so we don't need more one pool
If more than one place in the code submits tasks to the shared
pool, then waitTillDone() becomes unreliable.
Add a tagging mechanism, so different callsites can wait
on different sets of tasks.

Also try to protect our worker threads against exceptions from
the thread tasks code.

Change-Id: Idde664ab50008d31a2dd73910bb22f50e62ae22f
Reviewed-on: https://gerrit.libreoffice.org/27042
Tested-by: Jenkins <ci@libreoffice.org>
Reviewed-by: Noel Grandin <noelgrandin@gmail.com>
2016-07-18 06:49:09 +00:00
Ashod Nakashian
60e75fb276 tdf#98955 hardware_concurrency not ideal for thread pools
A new static member getPreferredConcurrency added to
comphelper::ThreadPool to return a configurable max
number of threads.

By default the new function returns the hardware_concurrency
value provided by std::thread. When MAX_CONCURRENCY envar is
defined, the return value is limited to whatever is set there.

Three call-sites that used std:🧵:hardware_concurrency
have been replaced with getPreferredConcurrency.

Unittests added to cover the functionality of the new member.

Unittests are capped to 4 threads.

Change-Id: I3332e393a88a5ed436316fa712ed920a4b37f4af
Reviewed-on: https://gerrit.libreoffice.org/26254
Tested-by: Jenkins <ci@libreoffice.org>
Reviewed-by: Ashod Nakashian <ashnakash@gmail.com>
2016-06-15 21:28:47 +00:00
Noel Grandin
d96c114171 clang-tidy modernize-make-shared
Change-Id: I3fa866bfb3093fc876474a9d9db29fe05dc2af3a
Reviewed-on: https://gerrit.libreoffice.org/25056
Tested-by: Jenkins <ci@libreoffice.org>
Reviewed-by: Noel Grandin <noelgrandin@gmail.com>
2016-05-18 06:58:44 +00:00
Noel Grandin
e8fd5a07ec update loplugin stylepolice to check local pointers vars
are actually pointer vars.

Also convert from regex to normal code, so we can enable this
plugin all the time.

Change-Id: Ie36a25ecba61c18f99c77c77646d6459a443cbd1
Reviewed-on: https://gerrit.libreoffice.org/24391
Tested-by: Jenkins <ci@libreoffice.org>
Reviewed-by: Noel Grandin <noelgrandin@gmail.com>
2016-04-26 10:55:58 +00:00
Noel Grandin
e1af7f0c43 clang-tidy modernize-loop-convert in c*
Change-Id: I77d2548f8be97792660761e6156cd24734a95aaf
2016-04-21 11:03:55 +02:00
Stephan Bergmann
fb4ce444c2 loplugin:nullptr (automatic rewrite)
Change-Id: Ibd0e6ae5e3243464b2484a009f2b4781bdaac471
2015-11-10 10:31:19 +01:00
Stephan Bergmann
b36963c0a6 Replace "SAL_OVERRIDE" with "override" in LIBO_INTERNAL_ONLY code
Change-Id: I2ea407acd763ef2d7dae2d3b8f32525523ac8274
2015-10-12 17:52:29 +02:00
Caolán McNamara
22b80ac8e2 boost->std
Change-Id: I3fd9e1599c5ad812879a58cf1dabbcd393105e1c
Reviewed-on: https://gerrit.libreoffice.org/18564
Tested-by: Jenkins <ci@libreoffice.org>
Reviewed-by: Caolán McNamara <caolanm@redhat.com>
Tested-by: Caolán McNamara <caolanm@redhat.com>
2015-09-14 12:54:25 +00:00
Caolán McNamara
a941df0f89 cppcheck: noExplicitConstructor
Change-Id: Ie2ae923ad4c1a66e779711de6ff05328ef144dac
2015-06-08 17:35:00 +01:00
Matúš Kukan
6f46cf4fe1 thread-pool: Initialize empty pools to start complete.
Otherwise waiting for completion if we push no work hangs.

Change-Id: I7103bdb779eb66a65cd8496091e72a0c65eb3567
2014-11-04 19:38:28 +00:00
Michael Meeks
68f912e896 thread-pool: fix waiting until all tasks are complete.
Change-Id: Iaf5a3bf28879f229a223a8760fd878f96958a53c
2014-10-31 18:13:05 +00:00
Michael Meeks
593a44a12d thread-pool: re-work termination semantics to avoid problems.
We want a pre-spun-up, shared thread-pool that doesn't get its
workers created & joined frequently.

Change-Id: I29081e3a3e3849ca30e63fd080ee3315d99cbe8d
2014-10-30 22:12:29 +00:00
Michael Meeks
62090f65b8 Move thread-pool down into comphelper for re-use elsewhere.
Change-Id: Ib27b8b1ccc07ff194035d6c2ef3d45c429e3cea1
2014-10-30 22:12:27 +00:00