On Wednesday 06 February 2013 12:38:12 Thiago Macieira wrote: > 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
wakeUp is called from many places, and without the lock. It seems to be called from QCoreApplicationPrivate::sendPostedEvents with the mutex locked, but that is a bug. We should unlock that mutex before calling wakeUp, as wakeUp is user code and should not be called with the mutex hold. > - it's only called *after* the event queue has been modified in postEvent > - processThreadWakeUp is only called *before* the event queue is inspected Yes, and that should garentee that QEvent get properly processed and that there is no race condition. > 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. Maybe there are other events? integration with a non-Qt event loop? -- Olivier Woboq - Qt services and support - http://woboq.com _______________________________________________ Development mailing list Development@qt-project.org http://lists.qt-project.org/mailman/listinfo/development
