diff options
author | Thiago Macieira <thiago.macieira@intel.com> | 2018-12-10 19:18:57 -0800 |
---|---|---|
committer | Thiago Macieira <thiago.macieira@intel.com> | 2019-02-27 04:26:13 +0000 |
commit | 197029b3d23237e61311019de1b63e3ce6720ed5 (patch) | |
tree | 4c2feb22f60cc9eb8ad7e8f5bca5c29628386f42 /src/corelib/kernel/qsystemsemaphore_posix.cpp | |
parent | 1500d2283e17be72226b2960a79039254887977f (diff) |
Make the QEventDispatcherWin32Private::interrupt flag atomic
Not entirely sure that this solves the problem reported in the bug
report, but here's the theory: the loop
633 while (!d->interrupt) {
...
710 }
has few calls that recurse back, so the compiler optimizer can assume
that the variable remains unchanged (not interrupted) in most of the
branches. Additionally, it can assume the variable did not change from
there to
712 // still nothing - wait for message or signalled objects
713 canWait = (!retVal
714 && !d->interrupt
715 && (flags & QEventLoop::WaitForMoreEvents));
Which causes canWait to be true, despite having been interrupted by
another thread.
Changing to an atomic does not force the reloading of the variable
(strictly speaking, would need volatile, but all atomic implementations
do reload now), but it solves the problem of data race, which was UB.
The equivalent variable in the Unix event dispatcher is atomic (commit
49d7e71f77f899c05e4b5187e8834dfcbddf4505 from 2013). I've reordered the
bool members so they're all together and reduce the amount of padding in
this class.
Fixes: QTBUG-72438
Change-Id: I4ac1156702324f0fb814fffd156f290df94dc4c7
Reviewed-by: Joerg Bornemann <joerg.bornemann@qt.io>
Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>
Diffstat (limited to 'src/corelib/kernel/qsystemsemaphore_posix.cpp')
0 files changed, 0 insertions, 0 deletions