[PATCH] thread-safe KQueuedDialog episode 2

[Sebastian Trueg]

we will get there... :)
new patch attached.

Matthias Kretz wrote:
> 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;
>>>>  }
> 
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: kdialog-thread-safe-queued-dialogs5.diff
URL: <http://mail.kde.org/pipermail/kde-core-devel/attachments/20081127/dba180f1/attachment.ksh>