[prev in list] [next in list] [prev in thread] [next in thread]
List: kde-core-devel
Subject: Re: [PATCH] thread-safe KQueuedDialog episode 2
From: Matthias Kretz <kretz () kde ! org>
Date: 2008-11-27 15:10:37
Message-ID: 200811271610.37590.kretz () kde ! org
[Download RAW message or body]
The changes all look good to me. Though I'm not sure they're enough to make it
thread-safe. What about concurrent accesses to KDialogQueue::queue? One thread
might be appending while the GUI thread calls pop.
On Thursday 27 November 2008 15:28:46 Sebastian Trueg wrote:
> you are of course perfectly right. I have no idea why I did not put that
> into the constructor. Will do. Other than that, patch ok?
>
> Matthias Kretz wrote:
> > Hi,
> >
> > On Thursday 27 November 2008 10:35:42 Sebastian Trueg wrote:
> >> KDialogQueue* KDialogQueue::self()
> >> {
> >> K_GLOBAL_STATIC(KDialogQueue, _self)
> >> +
> >> + // we want KDialogQueue to be thread-safe, i.e. it should be possible
> >> + // to queue dialogs from any thread, not only the GUI thread. For
> >> that + // to work, the KDialogQueue has to live in the GUI thread. Only
> >> then + // the queued connections used below will end up there.
> >> + if(_self->thread() != QApplication::instance()->thread() &&
> >> + _self->thread() == QThread::currentThread())
> >> + _self->moveToThread(QApplication::instance()->thread());
> >> +
> >
> > AFAICS you can move this code into the KDialogQueue constructor. That way
> > you're guaranteed to get a KDialogQueue object that is linked to the GUI
> > thread from all threads. Otherwise you have a race condition where two
> > threads access this function, the non-GUI thread creates the object and
> > the other thread starts to use it before the thread that created the
> > object moves it to the GUI thread.
> > Also you then have to do the thread affinity check only once instead of
> > for every access.
> >
> >> return _self;
> >> }
--
________________________________________________________
Matthias Kretz (Germany) <><
http://Vir.homelinux.org/
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic