Details
-
Bug
-
Resolution: Done
-
P1: Critical
-
5.0.0, 5.0.1
-
None
-
Gentoo i686. Reproduced on both Qt 5.0.0 and 5.0.1
-
f56829662308e84a578a3816cc054538f7d91e44
Description
We have a fairly complex Qt application where render thread occasionally goes to sleep and never wakes up (in Qt 5.0.0, this is in qquickthreadedwindowmanager.cpp:525). Inspecting it in gdb reveals that isRenderBlocked == false, despite the render thread having just set it to true right before it calls wait().
I believe this is the result of using a number of bit field variables without thread synchronization.
QQuickRenderThreadContextWindow declares a number of true/false variables as bitfields instead of plain old integers. e.g., this:
uint animationRunning: 1; uint isPostingSyncEvent : 1; uint isRenderBlocked : 1; uint isExternalUpdatePending : 1; uint syncAlreadyHappened : 1; uint inSync : 1; uint shouldExit : 1; uint hasExited : 1; uint isDeferredUpdatePosted : 1;
instead of this:
uint animationRunning; uint isPostingSyncEvent; uint isRenderBlocked; uint isExternalUpdatePending; uint syncAlreadyHappened; uint inSync; uint shouldExit; uint hasExited; uint isDeferredUpdatePosted;
Additionally, the class itself is accessed from two threads: the GUI thread and the internally managed render thread. A number of the bit field variables are read and written from both threads without being locked. For normal word-sized and word-aligned aligned variables, that's fine and thread safe. However, because the variables are declared as bit fields, writes are no longer atomic operations since a single word can be used to store up to 32 of those bit field variables (i.e., all of them). Setting a single bit variable requires setting all of them at once. That means if one thread does this:
isRenderBlocked = true;
and then another thread almost simultaneously sets any of the others, e.g.:
isDeferredUpdatePosted = true; isExternalUpdatePending = true; syncAlreadyHappened = false;
then it is possible that the first writing of isRenderBlocked gets wiped out by the second because the threads are not synchronized. I believe this is happening, and is causing the render thread to block forever on a condition variable and not wake up.
I've attached a simple C++ program that demonstrates how using bit fields messes with thread safety in an unintuitive way.
The fix that works for us is to change a bunch of the instance variables to be normal uint and not bit fields. Patch is attached.