threaded progress bar & deadlock ...
Michael Meeks
michael.meeks at collabora.com
Tue Dec 3 06:14:50 PST 2013
Hi there,
Turns out the silly deadlock we see on XLSX loading is a result of the
windows' "Yield" implementation being simply awful[1] ;-) The progress
bar will deadlock vs. the main-loop in this case.
So - what can we do ? :-) interestingly the 'Yield' drops the solar
mutex all over the place inside our filters at random places already
today - so I'm no longer concerned about that: which is encouraging.
Separately - we wanted to put the progress bar updates into a thread -
and we should do that in the main thread I think.
That's actually not that hard to do IMHO; so instead of
(sc/source/filter/oox/workbookfragment.cxx):
{
// Ideally no-one else but our worker threads can re-acquire that.
// potentially if that causes a problem we might want to extend
// the SolarMutex functionality to allow passing it around.
SolarMutexReleaser aReleaser;
aPool.waitUntilWorkersDone();
}
We can drop the comment - there is no new risk here. And instead of the
'SolarMutexReleaser' we should just run an inferior main-loop and we
don't even need any explicit locking - the SolarMutex should do that for
us ;-)
ProgressBarTimer aTimer;
while (mnWorkingThreads > 0) {
Application::Yield();
}
aPool.waitUntilWorkersdone();
And the ProgressBarTimer - should just update and re-render the timer
every 500ms or so.
As a side-effect / bonus this should let other LibreOffice applications
continue working while long sheet loading is occurring ;-) which might
be nice - but is no more risky than the current scattering of 'Yield's
we have everywhere.
Oh - and of course, in this mode - we need to tweak the
WorksheetGlobals::setRowModel() thing to not fiddle with the global oox
SubSegment::setPosition thing - but do that in the main loop. Instead I
guess we should have some atomically written sal_Int32 nStepsDone;
variable or something that the threads update and continue working ASAP
- without rendering anything - the 500ms timeout in the main thread will
handle that.
The mnWorkingThreads should be updated by the ThreadWorker when they
exit I guess.
How does that sound ?
hopefully it kills our windows deadlock without having to de-bong the
craziness in[1] for 4.2 :-)
At least that's one idea,
ATB,
Michael.
[1] - http://cgit.freedesktop.org/libreoffice/core/tree/vcl/win/source/app/salinst.cxx#n681
is reasonably amazing, not only the 'sleep (1);' but also the assumption
that the main thread is not (eg.) doing a thread.join(); on a thread trying
to do something with the GUI & => will never return.
--
michael.meeks at collabora.com <><, Pseudo Engineer, itinerant idiot
More information about the LibreOffice
mailing list