rhi: Be more graceful when one destroys a resource after the QRhi

One is a bad application or library in this case, but nonetheless
we should handle this more gracefully then just crashing due to
the QRhi already having been destroyed. Mainly because in Qt 5 one
could get away with the same: releasing OpenGL objects underneath,
for example, a QSGPlainTexture with no (or wrong) GL context did
not generate any user visible fatal errors. So we should not crash
in Qt 6 either with these code bases.

In debug builds or when QT_RHI_LEAK_CHECK is set, one will get the
unreleased resources warning printed in Qt 6, which is a step
forward compared to Qt 5. So there is still some indication that
something is badly designed, even if the application survives.

Task-number: QTBUG-95394
Pick-to: 6.2
Change-Id: I944f4f425ff126e7363a82aff926b280ccf1dfc3
Reviewed-by: Qt CI Bot <qt_ci_bot@qt-project.org>
Reviewed-by: Andy Nichols <andy.nichols@qt.io>

1 files changed, 12 insertions, 4 deletions

diff --git a/src/gui/rhi/qrhi.cpp b/src/gui/rhi/qrhi.cpp
index 4ae75b5df5..88fb19e5f3 100644
--- a/src/gui/rhi/qrhi.cpp
+++ b/src/gui/rhi/qrhi.cpp
@@ -4439,11 +4439,19 @@ QRhiImplementation::~QRhiImplementation()
     // release builds: opt-in
     static bool leakCheck = qEnvironmentVariableIntValue("QT_RHI_LEAK_CHECK");
 #endif
-    if (leakCheck && !resources.isEmpty()) {
-        qWarning("QRhi %p going down with %d unreleased resources that own native graphics objects. This is not nice.",
-                 q, int(resources.count()));
+    if (!resources.isEmpty()) {
+        if (leakCheck) {
+            qWarning("QRhi %p going down with %d unreleased resources that own native graphics objects. This is not nice.",
+                     q, int(resources.count()));
+        }
         for (QRhiResource *res : qAsConst(resources)) {
-            qWarning("  %s resource %p (%s)", resourceTypeStr(res), res, res->m_objectName.constData());
+            if (leakCheck)
+                qWarning("  %s resource %p (%s)", resourceTypeStr(res), res, res->m_objectName.constData());
+
+            // Null out the resource's rhi pointer. This is why it makes sense to do null
+            // checks in the destroy() implementations of the various resource types. It
+            // allows to survive in bad applications that somehow manage to destroy a
+            // resource of a QRhi after the QRhi itself.
             res->m_rhi = nullptr;
         }
     }