--headless broken with --enable-headless

[Michael Meeks]

Hi Stephan,

On Wed, 2012-12-12 at 13:12 +0100, Stephan Bergmann wrote:
> On 10/01/2012 07:40 PM, Riccardo Magliocchetti wrote:
> > nAcquireCount is always 0 and the patch above let me run two unoconv
> > testsuite runs in a row without valgrind logs.
> 
> It indeed looks like Application::Yield shall be routinely called with 
> the solar mutex unlocked, and that timer callbacks called from within 
> Yield shall be called with the solar mutex locked

	Hmm; that really seems broken to me - we shouldn't really call anything
in vcl without the solarmutex held IMHO.

> sal_gtk_timeout_dispatch (vcl/unx/gtk/app/gtkdata.cxx; when using the 
> GTK VCL plug-in) explicitly locks the solar mutex  before calling the 
> timer callback in a stack like

	Sure sure - I appreciate that; I'm not sure it's correct myself - it
may well be to work around the equivalent of a missing:

> Now, the interesting thing is that the Linux-specific CreateSalInstance 
> (called from ImplSVMain -> InitVCL before calling the application's Main 
> in ImplSVMain) explicitly locks the solar mutex thanks to 
> <http://cgit.freedesktop.org/libreoffice/core/commit/?id=852574f46f686a936a1b267e5780ca17d0f0d5ab> 
> "INTEGRATION: CWS ause0c2 (1.3.18); FILE MERGED: 2004/03/12 15:25:43 hjs 
> 1.3.18.1: #115868# anti freeze," while all the other variants of 
> CreateSalInstance apparently do not (incl. the one in 
> vcl/headless/headlessinst.cxx that is relevant when configured 
> --enable-headless, as is the case for this mail thread).

	That looks like the interesting bug to me.

>  That means that all of desktop::Desktop::Main -> Application::Execute -> 
> Application::Yield runs with the solar mutex locked on Linux (sans 
> --enable-headless configuration) and with the solar mutex unlocked 
> everywhere else.

	And that's what worries me - IMHO that is a bug; and one I'd like to
see fixed across platforms.

> The commit message for 852574f46f686a936a1b267e5780ca17d0f0d5ab is 
> unfortunately rather unhelpful, but temporarily reverting it and doing 
> "make check" on a Linux box starts to produce crashes within 
> desktop::Desktop::Main -> Application::Execute -> Application::Yield -> 
> ..., in code that apparently expects the solar mutex to be locked.

	Sure - the solar mutex should be protecting VCL during our startup /
bootstrap, and if we're not acquiring it as we create that thing
then ... life is going to be really bad IMHO.

	So - I propose something like the attached; at least for Beta2 - so we
can see what the effect is - I hope a positive one, particularly on
startup / migration / etc.

	Thoughts ?

		Michael.

-- 
michael.meeks at suse.com  <><, Pseudo Engineer, itinerant idiot
-------------- next part --------------
A non-text attachment was scrubbed...
Name: take-mutex-correctly.diff
Type: text/x-patch
Size: 1746 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/libreoffice/attachments/20121212/6052433e/attachment.bin>