Skip to content

fix: Abnormal hiding of the grouping window#526

Merged
deepin-bot[bot] merged 2 commits into
linuxdeepin:masterfrom
JWWTSL:master
May 6, 2026
Merged

fix: Abnormal hiding of the grouping window#526
deepin-bot[bot] merged 2 commits into
linuxdeepin:masterfrom
JWWTSL:master

Conversation

@JWWTSL
Copy link
Copy Markdown
Contributor

@JWWTSL JWWTSL commented Apr 28, 2026

log: GroupConfigOptDlg was created without setting a parent window. Consequently, the Window Manager (WM) was unaware that this dialog should be placed above the terminal window. When the user clicked the terminal area, the WM brought the terminal to the foreground, obscuring the dialog. Since the finished signal was not triggered, the terminal remained permanently frozen due to setEnabled(false).

pms: bug-358805

log: GroupConfigOptDlg was created without setting a parent window. Consequently, the Window Manager (WM) was unaware that this dialog should be placed above the terminal window. When the user clicked the terminal area, the WM brought the terminal to the foreground, obscuring the dialog. Since the finished signal was not triggered, the terminal remained permanently frozen due to setEnabled(false).

pms: bug-358805
@deepin-ci-robot
Copy link
Copy Markdown

deepin pr auto review

这段代码的修改主要是将 GroupConfigOptDlg 的构造函数调用从传递一个参数改为传递两个参数,第二个参数是 this 指针。这是一个非常关键的改进。

以下是针对这段代码的详细审查意见,包括语法逻辑、代码质量、代码性能和代码安全方面:

1. 语法逻辑

  • 审查结果:通过。
  • 分析:语法上没有错误。修改后的代码表明 GroupConfigOptDlg 的构造函数已被更新,现在接受一个 QObject* 类型的父对象指针(推测是 QWidget*QObject*)。这是 Qt 编程中的标准做法。

2. 代码质量

  • 审查结果:显著提升。
  • 分析
    • 对象所有权明确:在 Qt 中,将 this 作为父对象传递给子对象(dlg)是非常好的实践。这建立了明确的对象树关系。当父对象 RemoteManagementPanel 被销毁时,子对象 dlg 也会自动被销毁。
    • 防止内存泄漏:原代码 new GroupConfigOptDlg(groupName) 没有指定父对象。虽然在 lambda 表达式中处理了 finished 信号,但如果没有显式调用 delete 或者 deleteLater(),且父对象被销毁时,该对话框对象可能成为孤立的内存泄漏。修改后,Qt 的对象树机制会自动管理内存。
    • 代码一致性:这符合 Qt 的编程规范,使代码库更加统一和易于维护。

3. 代码性能

  • 审查结果:影响极小,正面。
  • 分析
    • 传递 this 指针仅仅是一次指针拷贝,性能开销可以忽略不计。
    • 由于建立了父子关系,对象销毁时的级联删除是 Qt 内部优化过的,不会带来明显的性能负担。

4. 代码安全

  • 审查结果:提升安全性。
  • 分析
    • 避免悬空指针:如果 RemoteManagementPanel 被销毁,而 dlg 没有父对象,dlg 可能继续存在。如果此时 dlg 尝试通过 connect 访问 this 的成员(虽然 lambda 是按值捕获的 this,但如果 dlg 内部持有对 panel 的引用),或者 dlg 触发信号试图通知已销毁的 panel,会导致未定义行为或崩溃。通过设置父对象,dlg 会随 panel 一起销毁,避免了这种危险情况。
    • 资源释放:确保了在异常或流程跳转情况下,对话框资源也能被正确回收。

改进建议

虽然这段修改已经解决了最核心的内存管理问题,但为了进一步完善代码,建议考虑以下几点:

  1. 窗口属性设置
    通常非模态对话框(Non-modal dialog,即 show() 而非 exec() 的对话框)建议设置 Qt::WA_DeleteOnClose 属性。这样当用户点击关闭按钮时,对话框会自动销毁,而不是仅仅隐藏。

    // 建议在 new 之后添加
    dlg->setAttribute(Qt::WA_DeleteOnClose);
  2. Lambda 捕获的安全性
    代码中使用了 [ = ] 进行值捕获。

    • 如果 dlg 设置了 WA_DeleteOnClose,当 finished 信号发出时,dlg 对象可能即将被销毁。在 lambda 内部访问 dlg 的成员通常是安全的(因为信号槽连接机制会保证执行顺序),但需谨慎。
    • 如果 lambda 中捕获了 this 指针(虽然这里是隐式捕获,因为是在成员函数中),且 this 被销毁,而信号因为队列连接延迟触发,可能会导致崩溃。不过 finished 信号通常是直接连接,风险较小。
  3. 智能指针(可选)
    如果不想依赖 Qt 的对象树来管理生命周期,或者该对话框有复杂的所有权转移需求,可以使用 QSharedPointerQScopedPointer。但在 Qt GUI 编程中,利用父子对象树通常是最推荐且最高效的方式,目前的修改已经足够好。

总结

这是一个高质量的修改。它通过引入父子对象关系,利用 Qt 的内存管理机制有效地解决了潜在的内存泄漏和悬空指针风险,符合 Qt 的最佳实践。建议配合设置 Qt::WA_DeleteOnClose 属性以达到完美的资源管理效果。

@deepin-ci-robot
Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: JWWTSL, lzwind

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@JWWTSL
Copy link
Copy Markdown
Contributor Author

JWWTSL commented May 6, 2026

/forcemerge

@deepin-bot deepin-bot Bot merged commit a24bf0b into linuxdeepin:master May 6, 2026
6 checks passed
@deepin-bot
Copy link
Copy Markdown
Contributor

deepin-bot Bot commented May 6, 2026

This pr force merged! (status: unknown)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants