fix: resolve occasional crash in TreeLandWindow destructor under treeland#1583
Conversation
treeland Remove the emission of stateChanged signal in TreeLandWindow destructor to prevent accessing partially destroyed objects. Refactor fullscreen state update logic into a dedicated method called from multiple locations (clear, handle remove) instead of using lambda connections. Added updateFullscreenState call in clear() to maintain correct state when all windows are removed. Log: Fixed crash when TreeLandWindow is destroyed under treeland Influence: 1. Test window management operations under treeland (open/close/ maximize/minimize) 2. Verify fullscreen state detection works correctly 3. Test rapid window creation and destruction 4. Test dock visibility changes during fullscreen transitions fix: 修复treeland下TreeLandWindow析构时偶发崩溃 移除TreeLandWindow析构函数中的stateChanged信号发射，避免访问已部分析构的 对象。将全屏状态更新逻辑重构为专用方法，在clear()和handle remove等多个位 置调用，替代lambda连接。在clear()中添加updateFullscreenState调用，确保移 除所有窗口时保持正确状态。 Log: 修复treeland下TreeLandWindow析构时的崩溃问题 Influence: 1. 在treeland环境下测试窗口管理操作（打开/关闭/最大化/最小化） 2. 验证全屏状态检测功能正常工作 3. 测试窗口的快速创建和销毁场景 4. 测试全屏切换过程中dock可见性变化
18202781743
requested review from
BLumia,
fly602,
mhduiy,
robertkill,
wjyrich and
yixinshark
Reviewer's GuideRefactors fullscreen state tracking in TreeLandWindowMonitor to use a shared helper method and avoid emitting stateChanged from the TreeLandWindow destructor, preventing crashes caused by accessing partially destroyed objects, while also ensuring fullscreen state is recalculated when windows are cleared or removed. Updated class diagram for TreeLandWindow and TreeLandWindowMonitor fullscreen logicclassDiagram
class AbstractWindow {
<<interface>>
+bool isFullscreen()
+signal stateChanged()
}
class TreeLandWindow {
+TreeLandWindow(uint32_t id, QObject *parent)
+~TreeLandWindow()
+uint32_t id()
+signal stateChanged()
}
class TreeLandWindowMonitor {
-QHash<ulong, QSharedPointer<TreeLandWindow>> m_windows
-QSharedPointer<AbstractWindow> m_dockPreview
-bool m_fullscreenState
+void clear()
+QPointer<AbstractWindow> getWindowByWindowId(ulong windowId)
+slot handleForeignToplevelHandleAdded()
+slot handleForeignToplevelHandleRemoved()
+slot updateFullscreenState()
+signal windowFullscreenChanged(bool fullscreen)
}
AbstractWindow <|-- TreeLandWindow
TreeLandWindowMonitor "1" o--> "*" TreeLandWindow
TreeLandWindowMonitor ..> AbstractWindow : uses
TreeLandWindowMonitor ..> TreeLandWindow : manages
TreeLandWindowMonitor ..> TreeLandWindow : connects stateChanged to updateFullscreenState
TreeLandWindowMonitor ..> DockPanel
class DockPanel {
+slot onWindowFullscreenChanged(bool fullscreen)
}
DockPanel ..> TreeLandWindowMonitor : listens windowFullscreenChanged
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
Program terminated with signal SIGSEGV, Segmentation fault. |
deepin pr auto review这段代码主要涉及 Wayland 窗口管理相关的逻辑，特别是在处理全屏状态变化时的重构。以下是对这段 diff 的详细审查意见： 1. 语法逻辑优点：
问题与建议：
2. 代码质量优点：
建议：
3. 代码性能分析：
4. 代码安全优点：
潜在风险点：
总结这是一次高质量的重构。主要改进点在于：
最终建议： |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- In
updateFullscreenState(), iterate withconst auto &window(or similar) instead of copying eachQSharedPointerto avoid unnecessary reference count churn. - Given
updateFullscreenState()is now called from multiple places, consider documenting in the header (e.g., brief comment) that it recalculates and emits fullscreen changes based onm_windows, so future callers know its side effects.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `updateFullscreenState()`, iterate with `const auto &window` (or similar) instead of copying each `QSharedPointer` to avoid unnecessary reference count churn.
- Given `updateFullscreenState()` is now called from multiple places, consider documenting in the header (e.g., brief comment) that it recalculates and emits fullscreen changes based on `m_windows`, so future callers know its side effects.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: 18202781743, wjyrich 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 |
3 participants
Remove the emission of stateChanged signal in TreeLandWindow destructor
to prevent accessing partially destroyed objects. Refactor fullscreen
state update logic into a dedicated method called from multiple
locations (clear, handle remove) instead of using lambda connections.
Added updateFullscreenState call in clear() to maintain correct state
when all windows are removed.
Log: Fixed crash when TreeLandWindow is destroyed under treeland
Influence:
maximize/minimize)
fix: 修复treeland下TreeLandWindow析构时偶发崩溃
移除TreeLandWindow析构函数中的stateChanged信号发射，避免访问已部分析构的
对象。将全屏状态更新逻辑重构为专用方法，在clear()和handle remove等多个位
置调用，替代lambda连接。在clear()中添加updateFullscreenState调用，确保移
除所有窗口时保持正确状态。
Log: 修复treeland下TreeLandWindow析构时的崩溃问题
Influence:
Summary by Sourcery
Resolve treeland window destruction crash by decoupling fullscreen state tracking from window lifetime and centralizing fullscreen updates in the monitor.
Bug Fixes:
Enhancements: