aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorLaszlo Agocs <laszlo.agocs@qt.io>2021-01-13 11:13:28 +0100
committerQt Cherry-pick Bot <cherrypick_bot@qt-project.org>2021-01-13 14:32:08 +0000
commite19e923f52217e82be9e11aadeacfbc917a00126 (patch)
tree6795c1ed3df4dc22da45845090dacdfad05e6d5f
parentbfa7b5a4641f387ca7efc2df7a9d04a03ca937db (diff)
Ensure the ninepatch image is detached
Some more contrived usages of nine patch images can uncover an issue with the nine patch node implementation when used together with the threaded render loop of Qt Quick: pixmapChanged generates a QImage that references external data, which then gets passed to the scenegraph in updatePaintNode during the scenegraph's sychronization phase. This pattern (QSGTexture from non-data-owning QImage) should be avoided in general, because when the gui thread gets unblocked and continues after the sync phase, it could invalidate the data the QImage points to. If now the independently running render thread happens to try accessing the data (still the QImage that got passed in in the sync phase), bad things may happen. While it can be difficult to reproduce an actual crash (without ASAN and such), logging with the specially crafted example code shows that the logic is problematic if non-owning QImages are involved: (the pointers are the QImage's constBits) - pixmapChange 0x1f1053a5954 [gui thread] - updatePaintNode 0x1f1053a5954 [render thread, gui blocked] - beforeRendering [render thread] - pixmapChange 0x1f1053e7424 [gui thread] - QSGPlainTexture bind/update 0x1f1053a5954 [render thread] // but 0x1f1053a5954 may be invalid at this point if not owned by the QImage passed to createTextureFromImage - frameSwapped [render thread] Fixes: QTBUG-88162 Change-Id: Id83d04fce668a3e05d150c086abdecc9d59e51e8 Reviewed-by: Mitch Curtis <mitch.curtis@qt.io> (cherry picked from commit 837b3795c237d20dfca4be46e10697e1cd300e60) Reviewed-by: Qt Cherry-pick Bot <cherrypick_bot@qt-project.org>
-rw-r--r--src/imports/controls/imagine/impl/qquickninepatchimage.cpp7
1 files changed, 7 insertions, 0 deletions
diff --git a/src/imports/controls/imagine/impl/qquickninepatchimage.cpp b/src/imports/controls/imagine/impl/qquickninepatchimage.cpp
index 1d6d60dd..a9da2929 100644
--- a/src/imports/controls/imagine/impl/qquickninepatchimage.cpp
+++ b/src/imports/controls/imagine/impl/qquickninepatchimage.cpp
@@ -449,6 +449,13 @@ QSGNode *QQuickNinePatchImage::updatePaintNode(QSGNode *oldNode, UpdatePaintNode
qsgnode_set_description(patchNode, QString::fromLatin1("QQuickNinePatchImage: '%1'").arg(d->url.toString()));
#endif
+ // The image may wrap non-owned data (due to pixmapChange). Ensure we never
+ // pass such an image to the scenegraph, because with a separate render
+ // thread the data may become invalid (in a subsequent pixmapChange on the
+ // gui thread) by the time the renderer gets to do something with the QImage
+ // passed in here.
+ image.detach();
+
QSGTexture *texture = window()->createTextureFromImage(image);
patchNode->initialize(texture, sz * d->devicePixelRatio, image.size(), d->xDivs, d->yDivs, d->devicePixelRatio);
return patchNode;