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