Repository navigation
fix: delete backdrop renderer on invalidated - #740
Conversation
Reviewer's GuideThe PR fixes an exit-time SIGSEGV by deleting the backdrop renderer synchronously when QSGRenderContext is invalidated, before its ShaderManager child is destroyed, and adds null guards plus a regression test for RHI window teardown. Sequence diagram for safe RHI renderer teardownsequenceDiagram
participant Window as QQuickWindow
participant Context as QSGRenderContext
participant Manager as RhiManager
participant Renderer as Renderer
participant Shader as ShaderManager
Window->>Context: invalidated()
Context-->>Manager: destroyRenderer()
Manager->>Renderer: delete renderer
Renderer->>Shader: clearCachedRendererData()
Context->>Shader: destroy ShaderManager
Window->>Manager: ~RhiManager()
Manager->>Manager: destroyRenderer()
Flow diagram for renderer invalidation guardsflowchart LR
A[QSGRenderContext invalidated] --> B["destroyRenderer()"]
B --> C["renderer = nullptr"]
C --> D{sync, preprocess, or render}
D -->|renderer or m_rhi unavailable| E[Return without rendering]
D -->|resources available| F[Continue RHI rendering]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've found 1 issue
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="src/private/dbackdropnode.cpp" line_range="385-386" />
<code_context>
}
bool preprocess(QRhiRenderTarget *rt, qreal &oldDPR, QRhiCommandBuffer* &oldCB, QSurface *forceSurface = nullptr) {
+ if (Q_UNLIKELY(!renderer || !m_rhi))
+ return false;
+
QRhiCommandBuffer *cb = nullptr;
</code_context>
<issue_to_address>
**Rendering crashes without an RHI**
When `QSGRhiSupport::createRhi()` returns no RHI during initialization or resource exhaustion, `preprocess()` returns false, but `RhiNode::render()` continues to access `RhiManager::rhi()` or `this->rhi->rhi()` with a null RHI, crashing the render thread and the app.
Make `RhiNode::render()` stop before accessing the manager’s RHI when no RHI was created.
</issue_to_address>| if (Q_UNLIKELY(!renderer || !m_rhi)) | ||
| return false; |
There was a problem hiding this comment.
🟡 Medium · Rendering crashes without an RHI
When QSGRhiSupport::createRhi() returns no RHI during initialization or resource exhaustion, preprocess() returns false, but RhiNode::render() continues to access RhiManager::rhi() or this->rhi->rhi() with a null RHI, crashing the render thread and the app.
Make RhiNode::render() stop before accessing the manager’s RHI when no RHI was created.
Prompt for AI agents
In `src/private/dbackdropnode.cpp` at lines 385-386:
**Rendering crashes without an RHI**
When `QSGRhiSupport::createRhi()` returns no RHI during initialization or resource exhaustion, `preprocess()` returns false, but `RhiNode::render()` continues to access `RhiManager::rhi()` or `this->rhi->rhi()` with a null RHI, crashing the render thread and the app.
Make `RhiNode::render()` stop before accessing the manager’s RHI when no RHI was created.1. RhiManager now connects to QSGRenderContext::invalidated (DirectConnection) and destroys the Renderer immediately in destroyRenderer(), rather than waiting until RhiManager's destructor runs during QObject child cleanup. 2. Renderer holds a reference to QSGRenderContext's ShaderManager child; deleting the renderer after that child is gone causes a SIGSEGV in ~Renderer -> ShaderManager::clearCachedRendererData. Handling invalidated matches Qt's QSGRhiLayer pattern. 3. sync(), preprocess(), and render() now bail out when renderer (or m_rhi in preprocess) is null, preventing operations on a destroyed RHI context. 4. context/renderer are initialized to nullptr in the constructor, and destroyRenderer() is reused from both invalidated and ~RhiManager. 5. Added regression test destroyWithBackdropNodeOnRhi, which renders a frame with DQuickBackdropBlitter then destroys the window; it crashed before this fix and exits cleanly after. Log: Fixed exit crash caused by DBackdropBlitter renderer lifecycle Influence: 1. Close windows in existing InWindowBlur demos/apps and confirm no crash on the default RHI rendering path. 2. Run related scenes with the software scene graph backend to confirm the guards preserve behavior when RhiManager is not created. 3. On Qt6, exercise window teardown with OpenGL/Vulkan/D3D/Metal RHI backends, with and without DTK_DISABLE_INWINDOWBLUR. 4. Run the new regression test ut_DQuickInWindowBlur.destroyWithBackdropNodeOnRhi and verify it passes. fix: 在 invalidated 时销毁 backdrop renderer 1. RhiManager 现在以直连连接监听 QSGRenderContext::invalidated,在信号 发出时立即通过 destroyRenderer() 销毁 Renderer,不再等到 RhiManager 析构 (QObject 子对象清理阶段)才执行。 2. Renderer 持有 QSGRenderContext 的 QObject 子对象 ShaderManager 的 引用;在 ShaderManager 销毁后才 delete Renderer,会在 ~Renderer → ShaderManager::clearCachedRendererData 中访问已释放内存并崩溃。Qt 对 QSGRhiLayer 也是同样处理。 3. sync()、preprocess()、render() 在 renderer(preprocess 中还包含 m_rhi)为空时直接返回,避免在已销毁的 RHI context 上继续渲染。 4. 构造函数将 context/renderer 初始化为 nullptr,删除逻辑集中到 destroyRenderer(),析构函数也调用它。 5. 新增回归用例 destroyWithBackdropNodeOnRhi:先通过 DQuickBackdropBlitter 渲染一帧再销毁窗口;修复前会崩溃,修复后可正常 退出。 Log: 修复 DBackdropBlitter 渲染器生命周期导致的退出崩溃 Influence: 1. 运行已有的 InWindowBlur 示例/应用并关闭窗口,确认默认 RHI 渲染路径不 崩溃。 2. 使用软件场景图后端运行相关场景,确认未创建 RhiManager 时保护逻辑不影 响原有行为。 3. 在 Qt6 下分别使用 OpenGL/Vulkan/D3D/Metal RHI 后端测试窗口销毁,并分 别设置/不设置 DTK_DISABLE_INWINDOWBLUR。 4. 运行新增回归用例 ut_DQuickInWindowBlur.destroyWithBackdropNodeOnRhi, 确认通过。 PMS: TASK-397773
deepin pr auto review🤖 AI 代码审查报告📊 总体评价
🔍 详细分析1. 语法逻辑 ✅评价: 语法正确,逻辑清晰 ✅ 通过 潜在问题: 建议: ['无需修改,语法逻辑完全正确。'] 2. 代码质量 ✅评价: 代码结构清晰,注释完整 ✅ 通过 潜在问题: 建议: ['无需修改,代码质量和注释完整度均达标。'] 3. 代码性能 ✅评价: 性能良好,资源使用合理 ✅ 通过 潜在问题: 建议: ['无需修改,性能表现良好。'] 4. 代码安全 🔒评价: 存在0个安全漏洞 ✅ 通过
安全漏洞详情: 建议: ['无需修改,代码安全合规。'] 💡 改进建议代码示例// 当前代码已正确修复了生命周期问题,无需额外修改。
// 核心修复模式如下(已存在于提交中):
//
// 在 RhiManager 构造函数中连接 invalidated 信号:
// connect(context, &QSGRenderContext::invalidated,
// this, &RhiManager::destroyRenderer, Qt::DirectConnection);
//
// 在 sync/preprocess/render 中增加空指针保护:
// if (Q_UNLIKELY(!renderer))
// return;
//
// destroyRenderer() 方法确保原子销毁:
// void destroyRenderer() {
// delete renderer;
// renderer = nullptr;
// }本报告由 AI 代码审查工具自动生成 |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: 18202781743, caixr23 The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
/merge |
|
This pr cannot be merged! (status: unstable) |
|
/forcemerge |
|
This pr force merged! (status: unstable) |
Log: Fixed exit crash caused by DBackdropBlitter renderer lifecycle
Influence:
fix: 在 invalidated 时销毁 backdrop renderer
退出。
Log: 修复 DBackdropBlitter 渲染器生命周期导致的退出崩溃
Influence:
PMS: TASK-397773
Summary by Sourcery
Fix backdrop renderer teardown so windows close safely after RHI rendering contexts are invalidated.
Bug Fixes:
Enhancements:
Tests: