fix: prevent crash from invalid indexes in window monitoring#1584
fix: prevent crash from invalid indexes in window monitoring#158418202781743 merged 1 commit intolinuxdeepin:masterfrom
Conversation
Add validity checks for model indexes and window pointers to prevent crashes when accessing data from window monitoring models. The crash occurred in treeland environment due to accessing invalid indexes or null window pointers, particularly in the `data()` methods of `AbstractWindowMonitor` and `RoleCombineModel`, and when matching windows in `TaskManager::handleWindowAdded`. Log: Fixed a crash in the dock's window monitoring system by adding null and index validity checks Influence: 1. Test dock stability under treeland by switching between multiple windows rapidly 2. Verify dock does not crash when adding/removing windows dynamically 3. Test with invalid model states (empty models, invalid indexes) 4. Verify window matching still works correctly for active apps 5. Test combination models with various role mappings fix: 修复窗口监控中无效索引导致的崩溃问题 在窗口监控模型中增加模型索引和窗口指针的有效性检查，防止因访问 无效索引或空窗口指针导致崩溃。该崩溃在 treeland 环境下偶发，主要 由 `AbstractWindowMonitor` 和 `RoleCombineModel` 的数据访问方法以及 `TaskManager::handleWindowAdded` 中的窗口匹配逻辑引起。 Log: 修复了 dock 窗口监控系统中因缺少空指针和索引有效性检查导致的崩溃 问题 Influence: 1. 在 treeland 环境下快速切换多个窗口测试 dock 稳定性 2. 验证动态添加/删除窗口时 dock 不会崩溃 3. 测试空模型和无效索引等异常状态 4. 验证活跃应用的窗口匹配功能正常 5. 测试多种角色映射的组合模型
18202781743
requested review from
BLumia,
fly602,
mhduiy,
robertkill,
wjyrich and
yixinshark
|
#0 0x00007fdcf953b36f in dock::AbstractWindowMonitor::data (this=0x55c0203c0010, index=..., role=257) at /home/deepin/dde-shell/panels/dock/taskmanager/abstractwindowmonitor.cpp:86 #8 0x00007fdcf95955ee in QtPrivate::FunctorCall<QtPrivate::IndexesList<0>, QtPrivate::List<QPointerdock::AbstractWindow >, void, void (dock::TaskManager::)(QPointerdock::AbstractWindow)>::call #16 0x00007fdcf95a22e2 in QtPrivate::FunctorCallBase::call_internal<void, QtPrivate::FunctorCall<QtPrivate::IndexesList<>, QtPrivate::List<>, void, void (dock::TreeLandWindowMonitor::)()>::call(void #18 0x00007fdcf95a10fc in QtPrivate::FunctionPointer<void (dock::TreeLandWindowMonitor::)()>::call<QtPrivate::List<>, void>(void (dock::TreeLandWindowMonitor::)(), dock::TreeLandWindowMonitor*, void**) |
Reviewer's GuideAdds defensive validity checks around model indexes and window pointers in the dock’s window monitoring and task management code to prevent crashes when accessing invalid data, especially in treeland environments. Sequence diagram for TaskManager::handleWindowAdded with index validity checkssequenceDiagram
participant TM as TaskManager
participant WM as AbstractWindow
participant AAM as ActiveAppModel
TM->>TM: handleWindowAdded(window)
TM->>WM: id()
WM-->>TM: winId
alt activeAppModel exists and has rows
TM->>AAM: rowCount()
AAM-->>TM: count
alt count > 0
TM->>AAM: index(0, 0)
AAM-->>TM: startIndex
alt startIndex is valid
TM->>AAM: match(startIndex, WinIdRole, winId)
AAM-->>TM: res
else startIndex is invalid
TM-->>TM: skip match
end
else no rows
TM-->>TM: skip match
end
else no activeAppModel
TM-->>TM: skip match
end
Class diagram for updated window monitoring and task management guardsclassDiagram
class RoleCombineModel {
+QVariant data(QModelIndex index, int role) const
+int columnCount(QModelIndex parent) const
+QAbstractItemModel* sourceModel() const
-QHash<QPair<int,int>, QPair<int,int>> m_indexMap
-QHash<int,int> m_minorRolesMap
-QAbstractItemModel* m_minor
}
class AbstractWindowMonitor {
+QVariant data(QModelIndex index, int role) const
+void requestWindowsView(QModelIndexList indexes) const
-QVector<QPointer<AbstractWindow>> m_trackedWindows
}
class TaskManager {
+void handleWindowAdded(QPointer<AbstractWindow> window)
-QAbstractItemModel* m_activeAppModel
}
class AbstractWindow {
+quint64 id() const
}
RoleCombineModel --> QAbstractItemModel : uses sourceModel
RoleCombineModel --> QAbstractItemModel : uses m_minor
AbstractWindowMonitor --> AbstractWindow : tracks via m_trackedWindows
TaskManager --> AbstractWindow : parameter window
TaskManager --> QAbstractItemModel : uses m_activeAppModel
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
deepin pr auto review这段代码 diff 主要针对 Qt 模型（Model）的数据访问安全性进行了改进，同时也更新了版权年份。以下是对修改内容的详细审查和改进建议： 1. 代码逻辑与安全性审查优点：
潜在问题与改进建议： A.
|
![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 AbstractWindowMonitor::data(), the combination of the explicit bounds check and using m_trackedWindows.value(pos, nullptr) is redundant; you can either rely on value() with a default and drop the size check, or keep the explicit check and use operator[] for a slightly clearer flow.
- In TaskManager::handleWindowAdded(), consider adding a null check for the QPointer before calling window->id() to make the function robust against a potentially cleared QPointer.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In AbstractWindowMonitor::data(), the combination of the explicit bounds check and using m_trackedWindows.value(pos, nullptr) is redundant; you can either rely on value() with a default and drop the size check, or keep the explicit check and use operator[] for a slightly clearer flow.
- In TaskManager::handleWindowAdded(), consider adding a null check for the QPointer<AbstractWindow> before calling window->id() to make the function robust against a potentially cleared QPointer.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
Add validity checks for model indexes and window pointers to prevent
crashes when accessing data from window monitoring models. The crash
occurred in treeland environment due to accessing invalid indexes
or null window pointers, particularly in the
data()methods ofAbstractWindowMonitorandRoleCombineModel, and when matchingwindows in
TaskManager::handleWindowAdded.Log: Fixed a crash in the dock's window monitoring system by adding null
and index validity checks
Influence:
windows rapidly
fix: 修复窗口监控中无效索引导致的崩溃问题
在窗口监控模型中增加模型索引和窗口指针的有效性检查，防止因访问
无效索引或空窗口指针导致崩溃。该崩溃在 treeland 环境下偶发，主要
由
AbstractWindowMonitor和RoleCombineModel的数据访问方法以及TaskManager::handleWindowAdded中的窗口匹配逻辑引起。Log: 修复了 dock 窗口监控系统中因缺少空指针和索引有效性检查导致的崩溃
问题
Influence:
Summary by Sourcery
Prevent crashes in the dock window monitoring system by validating model indexes and tracked window pointers before accessing data.
Bug Fixes:
Enhancements:
Chores: