Details
-
Bug
-
Resolution: Unresolved
-
P3: Somewhat important
-
None
-
5.14.2
-
None
-
macOS,
Windows
Description
We are seeing quite a lot (~1% of users daily) crashes inside QOffscreenSurface::setScreen. I decided to take a look, and I believe it's coming from the fact that if the QOffscreenSurface is moved to a thread other than the main thread, then the logic within QOffscreenSurface::screenDestroyed is broken.
What I can see, is that in QOffscreenSurface::screenDestroyed we call
setScreen(nullptr). And then in setScreen, we will disconnect from the current screen, and re-create the screen on the now main screen. But this will all happen on the background thread is the QOffscreenSurface is running there. This is because the destroyed slot from the QScreen object will happen async in this case, so we'll run screenDestroyed and hence setScreen(nullptr) after the QScreen has died.
So, if a QOffscreenSurface is running on a background thread, and running on a screen which is subsequently removed from the system by e.g. unplugging a monitor, then we will have a data race.
This is confirmed by a very useful ASan trace I just got:
=================================================================
==10341==ERROR: AddressSanitizer: heap-use-after-free on address 0x6020000bfa10 at pc 0x00016d0b79ca bp 0x70000b38a070 sp 0x70000b38a068
READ of size 8 at 0x6020000bfa10 thread T142
#0 0x16d0b79c9 in QObject::disconnect(QObject const*, char const*, QObject const*, char const*) qobject.cpp:3121
#1 0x16dcd46f1 in QOffscreenSurface::setScreen(QScreen*) qoffscreensurface.cpp:348
#2 0x16e9fe62b in QOffscreenSurface::qt_static_metacall(QObject*, QMetaObject::Call, int, void**) moc_qoffscreensurface.cpp:83
#3 0x16d0a5653 in QObject::event(QEvent*) qobject.cpp:1339
#4 0x171cfce36 in QApplicationPrivate::notify_helper(QObject*, QEvent*) qapplication.cpp:3685
#5 0x171d01480 in QApplication::notify(QObject*, QEvent*) qapplication.cpp
#6 0x10e9334b8 in <redacted>
#7 0x16d00e18c in QCoreApplication::notifyInternal2(QObject*, QEvent*) qcoreapplication.cpp:1075
#8 0x16d012eae in QCoreApplicationPrivate::sendPostedEvents(QObject*, int, QThreadData*) qcoreapplication.cpp:1815
#9 0x16d14f2f2 in QEventDispatcherUNIX::processEvents(QFlags<QEventLoop::ProcessEventsFlag>) qeventdispatcher_unix.cpp:466
#10 0x16d00005a in QEventLoop::exec(QFlags<QEventLoop::ProcessEventsFlag>) qeventloop.cpp:225
#11 0x16c9ce5b6 in QThread::exec() qthread.cpp:536
#12 0x16c9d180e in QThreadPrivate::start(void*) qthread_unix.cpp:342
#13 0x7fff6b0fe108 in _pthread_start (libsystem_pthread.dylib:x86_64+0x6108)
#14 0x7fff6b0f9b8a in thread_start (libsystem_pthread.dylib:x86_64+0x1b8a)
I've been trying to figure out the best way to solve this. But essentially, because QOffscreenSurface::create is not safe to run on a background thread, neither is QOffscreenSurface::setScreen, so neither is QOffscreenSurface itself because this is called arbitrarily when the QScreen dies.
I am going to come up with a workaround in our app to listen to the screen removed event, and pause things, then move the QOffscreenSurface to the main thread, then back again. But I'd appreciate some Qt eyes here to see if we can fix Qt itself.