Re: [Libreoffice] Solving an introduced runtime dependency on unreleased Qt version (KDE4 vcl backend)

Hi Lubos,

On Tue, 2011-11-22 at 18:26 +0100, Lubos Lunak wrote:
>  Since 3.4 at least, when run with KDE4 integration, LO kind of happens to 
> have a runtime dependency on a yet unreleased Qt version, otherwise LO will
> abort during some reasonably commonplace operations

        Urk.
        
> That doesn't quite fit with LO, which occassionally will do UI stuff even 
> from other threads.

        Right - but we guarentee that only one thread at a time will do that -
protected by the solar mutex. So there should never be two threads
concurrently inside the X libraries or the Qt toolkit.

        Are you suggesting that Qt stores the thread-id of the thread it is
initialized in, and then compares that all over the shop to check it is
the same one ? If so, that seems heinously broken - is there a way we
can turn that off more generally ?, if not - we shouldn't have a problem
- vcl should -never- enter Qt or X from more than one thread
concurrently.

> I also don't know how exactly thread safety is supposed to work in LO, so I 
> cannot quite guess. I've been told that VCL only works with Solar mutex 
> grabbed

        Right. Of course, sometimes there are bugs - and people may forget to
take the solar mutex ;-) but that is easier to fix - we need to add
that.

>  which means that Qt usage from it should be safe, if the way to 
> avoid the QPixmap check is avoided. But there are other UI elements in LO, 
> such as the fpicker, which AFAIK are not triggered using VCL and I don't know 
> how that is supposed to work.

        IIRC the fpicker for KDE is (was) an out-of-process helper
(program/kdefilepicker), so that should not be an issue.

>  What do you suggest to do about it?
>
> PS: I've attached a backport of the Qt change to 
> https://bugs.freedesktop.org/show_bug.cgi?id=40298 .

        Reading the patch, we shouldn't need to XInitThreads, IIRC vcl does
this for us. The real breakage looks like:

-    if (qApp->thread() != QThread::currentThread()) {

        all instances of that in Qt look highly suspect to me :-)

        Working around them, particularly on older systems may be a problem.
Short of having a "Qt thread" - and proxying all API calls (somehow)
into that in order to dispatch them with a given threadid might be one
approach [ but it is a ton of work I suspect ]. 

        Wait - now I read the QObject header file - each QObject seems to have
this thread property.

        I suspect that what we really want to do is to do a:

        qApp->moveToThread(QThread::currentThread());

        before we start calling Qt methods each time. Hopefully that is
reasonably efficient (surely it's just a member variable).

        To do that I would do something similar to the GtkHookedYieldMutex
[ rather more complicated than what you want ], and pass a pointer to
your SalYieldMutex sub-class to the X11SalInstance constructor. You'd
want to switch the thread to the currentThread pots first successful
acquire I suspect.

        Might be simpler :-)

        HTH,

                Michael.

-- 
michael.meeks@suse.com  <><, Pseudo Engineer, itinerant idiot