fix: prevent crash when treeland restarts in taskbar#1620
Conversation
Reviewer's guide (collapsed on small PRs)Reviewer's GuideIntroduces a model-safe way to clear tracked windows in the taskbar window monitors and wires it into both TreeLand and X11 clear() flows to prevent use-after-free crashes during compositor restarts. Sequence diagram for clear() with model-safe window resetsequenceDiagram
participant Monitor as TreeLandWindowMonitor
participant Model as AbstractWindowMonitor
participant View as TaskbarView
View->>Monitor: clear()
activate Monitor
Monitor->>Model: clearTrackedWindows()
activate Model
Model->>Model: beginResetModel()
Model->>Model: m_trackedWindows.clear()
Model->>Model: endResetModel()
deactivate Model
Monitor->>Monitor: m_windows.clear()
deactivate Monitor
View->>Model: dataChanged / rowCount
Model-->>View: updated window list (empty)
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 left some high level feedback:
- Consider whether
clearTrackedWindows()should be responsible for any additional cleanup beyond clearingm_trackedWindows(e.g., emitting the same signals or performing the same side effects asdestroyWindow()), to keep model state changes consistent regardless of whether windows are removed individually or in bulk. - If
clearTrackedWindows()is only intended to be used by concrete monitor implementations (likeTreeLandWindowMonitorandX11WindowMonitor), you might want to restrict its visibility (e.g.,protected) to avoid exposing it as part of the public API surface unnecessarily.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider whether `clearTrackedWindows()` should be responsible for any additional cleanup beyond clearing `m_trackedWindows` (e.g., emitting the same signals or performing the same side effects as `destroyWindow()`), to keep model state changes consistent regardless of whether windows are removed individually or in bulk.
- If `clearTrackedWindows()` is only intended to be used by concrete monitor implementations (like `TreeLandWindowMonitor` and `X11WindowMonitor`), you might want to restrict its visibility (e.g., `protected`) to avoid exposing it as part of the public API surface unnecessarily.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
The crash occurs because when treeland restarts, the clear() method in TreeLandWindowMonitor clears internal window lists but fails to reset the tracked windows model. When treeland restart triggers a cleanup followed by re-initialization, the model still holds stale references to windows that have been destroyed, leading to a use-after-free crash when the model attempts to access the window data during view updates. Added clearTrackedWindows() method to properly clear all tracked windows from the model using beginResetModel/endResetModel, and called it at the start of clear() in both TreeLandWindowMonitor and X11WindowMonitor to ensure the model is reset before internal lists are cleared. This ensures the model and view are properly synchronized, preventing access to destroyed window objects. Log: fix treeland restart crash Influence: 1. Test taskbar behavior when treeland restarts unexpectedly 2. Verify taskbar shows correct windows after treeland restart 3. Test taskbar with no windows open during treeland restart 4. Verify no crash when multiple rapid treeland restarts occur 5. Test X11 window monitor clear function for regressions 6. Verify taskbar preview windows still work correctly after restart fix: 修复任务栏在treeland重启时崩溃的问题 当treeland重启时,TreeLandWindowMonitor中的clear()方法清除了内部窗口列 表,但未能重置跟踪窗口的模型。treeland重启触发清理后重新初始化时,模型 仍然持有已被销毁窗口的陈旧引用,导致视图更新时访问已释放的窗口对象引发崩 溃。新增clearTrackedWindows()方法,通过beginResetModel/endResetModel正确 清除模型中的所有跟踪窗口,并在TreeLandWindowMonitor和X11WindowMonitor的 clear()方法开始时调用该方法,确保在清除内部列表之前重置模型。这保证了模 型和视图同步一致,防止访问已销毁的窗口对象。 Log: 修复treeland重启崩溃问题 Influence: 1. 测试treeland异常重启时任务栏的行为 2. 验证treeland重启后任务栏显示正确的窗口 3. 测试treeland重启时没有打开任何窗口的情况 4. 验证多次快速重启treeland不会导致崩溃 5. 测试X11窗口监视器的清除功能是否有回归问题 6. 验证重启后任务栏预览窗口仍然正常工作
deepin pr auto review你好!我是CodeGeeX。我已仔细审查了你提供的Git Diff。本次修改的主要目的是在清理窗口监控器时,正确地通知Qt的Model/View系统模型正在重置,以避免视图与底层数据不同步。 总体来说,这次修改的方向是正确的,使用了 以下是详细的审查意见: 1. 代码逻辑与内存安全 (严重问题)问题: 在
改进建议:
2. 代码性能 (建议优化)问题: 在
改进建议: 移除 3. 代码安全与规范 (版权声明)问题: 头文件中的版权声明从
改进建议: 如果是修改文件,通常规范的做法是追加当前年份,例如 改进后的代码建议基于以上分析,我为你提供改进后的代码实现: 1.
|
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: 18202781743, BLumia 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 |
|
/forcemerge |
|
This pr force merged! (status: blocked) |
The crash occurs because when treeland restarts, the clear() method in
TreeLandWindowMonitor clears internal window lists but fails to reset
the tracked windows model. When treeland restart triggers a cleanup
followed by re-initialization, the model still holds stale references
to windows that have been destroyed, leading to a use-after-free crash
when the model attempts to access the window data during view updates.
Added clearTrackedWindows() method to properly clear all tracked windows
from the model using beginResetModel/endResetModel, and called it at
the start of clear() in both TreeLandWindowMonitor and X11WindowMonitor
to ensure the model is reset before internal lists are cleared. This
ensures the model and view are properly synchronized, preventing access
to destroyed window objects.
Log: fix treeland restart crash
Influence:
fix: 修复任务栏在treeland重启时崩溃的问题
当treeland重启时,TreeLandWindowMonitor中的clear()方法清除了内部窗口列
表,但未能重置跟踪窗口的模型。treeland重启触发清理后重新初始化时,模型
仍然持有已被销毁窗口的陈旧引用,导致视图更新时访问已释放的窗口对象引发崩
溃。新增clearTrackedWindows()方法,通过beginResetModel/endResetModel正确
清除模型中的所有跟踪窗口,并在TreeLandWindowMonitor和X11WindowMonitor的
clear()方法开始时调用该方法,确保在清除内部列表之前重置模型。这保证了模
型和视图同步一致,防止访问已销毁的窗口对象。
Log: 修复treeland重启崩溃问题
Influence:
Summary by Sourcery
Prevent taskbar crashes when window monitors are cleared by resetting the tracked window model before internal window lists are cleared.
Bug Fixes:
Enhancements: