From 837b3795c237d20dfca4be46e10697e1cd300e60 Mon Sep 17 00:00:00 2001 From: Laszlo Agocs Date: Wed, 13 Jan 2021 11:13:28 +0100 Subject: 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] Pick-to: 6.0 5.15 Fixes: QTBUG-88162 Change-Id: Id83d04fce668a3e05d150c086abdecc9d59e51e8 Reviewed-by: Mitch Curtis --- src/imports/controls/imagine/impl/qquickninepatchimage.cpp | 7 +++++++ 1 file changed, 7 insertions(+) (limited to 'src') 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; -- cgit v1.2.3