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 <mitch.curtis@qt.io>

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;