[Development] Reviews needed before android integration in two weeks

[Thiago Macieira]

On quarta-feira, 6 de fevereiro de 2013 21.13.36, Olivier Goffart wrote:
> That said, it is probably a good idea not to call select at all when it is
> not  needed.  But with that patch, you also make calls to write even when
> it is not needed, while before there was only call to write.

The atomic is there to avoid a write to the pipe/eventfd when it has already 
been written to. That is, in QEventDispatcherUNIX::wakeUp, it saves a syscall 
if the target thread has already been told to wake up.

In QEventDispatcherUNIXPrivate::processThreadWakeUp, it will read from the 
pipe/eventfd and then reset the atomic back to zero, indicating that a write 
is necessary again.

Here's how an event is posted (thread A is the thread that does dispatching, 
thread B is the one posting the event):

 - thread A is awake in QEventDispatcherUNIXPrivate::processThreadWakeUp
 - thread B is in QEventDispatcherUNIX::wakeUp
 - thread A has just returned from read(2) but hasn't reset the atomic yet
 - (1) thread B is checking the atomic and finds it at 1
 - (2) thread A resets the atomic
 - thread B will then proceed without writing to the eventfd/pipe

All events in a given thread happen in the order that they are listed. Events 
in separate threads can happen in any order relative to each other, *except* 
for (1) and (2): must happen in that order because the reverse is not 
possible.

The above could be a problem, but isn't because it depends on external 
factors. Namely:

 - wakeUp() is only called from qcoreapplication.cpp with the event queue 
   mutex locked
 - it's only called *after* the event queue has been modified in postEvent
 - processThreadWakeUp is only called *before* the event queue is inspected

That means these two events are necessarily ordered:

 - thread B adds an event to thread A's event queue
   [happens before "thread B is in QEventDispatcherUnix::wakeUp",
    which happens before "thread B is checking the atomic and finds it at 1"]
 - thread A processes its event queue
   [happens after "thread A resets the atomic"]

This proves that the event posting is correct. Empirical evidence (namely, the 
lack of bug reports concerning this problem) also supports that conclusion.

If you're finding a problem, its root cause must be elsewhere and must be dealt 
there.

-- 
Thiago Macieira - thiago.macieira (AT) intel.com
  Software Architect - Intel Open Source Technology Center
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 190 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.qt-project.org/pipermail/development/attachments/20130206/f2a9c9aa/attachment.sig>