fix: fix crash caused by DockGroupModel destruction issue #1416
+8
−5
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The crash occurred due to improper memory management of DockGroupModel objects. Previously, m_groupModel was managed using QScopedPointer, which could lead to premature deletion when the model was still in use. The issue was fixed by changing to QObject parent-child relationship management. 1. Changed m_globalModel from raw pointer to QPointer for safer pointer tracking 2. Changed m_groupModel from QScopedPointer to raw pointer with QObject parent-child relationship 3. Modified DockGroupModel constructor to pass 'this' as parent parameter, enabling Qt's automatic object tree management 4. Added nullptr initialization for m_groupModel in constructor 5. Added null check in roleNames() method to prevent crashes when m_globalModel is null 6. Removed unnecessary reset() call when switching to split mode This ensures proper lifetime management where DockGroupModel instances are automatically deleted when their parent DockItemModel is destroyed, preventing use-after-free crashes. Log: Fixed application crash when switching between window grouping modes Influence: 1. Test switching between grouped and split window modes multiple times 2. Verify no crashes occur during dock item model operations 3. Test application stability when closing windows with grouped items 4. Verify roleNames() method handles null m_globalModel gracefully 5. Test memory management during model destruction scenarios fix: 修复DockGroupModel析构问题导致的崩溃 崩溃是由于DockGroupModel对象的内存管理不当引起的。之前使用QScopedPointer 管理m_groupModel，可能导致模型仍在使用时被提前删除。通过改为使用QObject 父子关系管理解决了此问题。 1. 将m_globalModel从原始指针改为QPointer，实现更安全的指针跟踪 2. 将m_groupModel从QScopedPointer改为原始指针，使用QObject父子关系管理 3. 修改DockGroupModel构造函数，传递'this'作为父参数，启用Qt的自动对象树 管理 4. 在构造函数中添加m_groupModel的nullptr初始化 5. 在roleNames()方法中添加空检查，防止m_globalModel为空时崩溃 6. 切换到分屏模式时移除不必要的reset()调用 这确保了正确的生命周期管理，当父对象DockItemModel被销毁时， DockGroupModel实例会自动删除，防止了使用已释放内存导致的崩溃。 Log: 修复了在切换窗口分组模式时应用程序崩溃的问题 Influence: 1. 测试多次切换分组和分屏窗口模式 2. 验证在执行停靠栏项目模型操作时不会发生崩溃 3. 测试关闭包含分组项目的窗口时的应用程序稳定性 4. 验证roleNames()方法能正确处理空的m_globalModel 5. 测试模型销毁场景下的内存管理 PMS: BUG-321495
18202781743
requested review from
BLumia,
fly602,
mhduiy,
robertkill,
wjyrich and
yixinshark
Contributor
Author
|
Reviewer's guide (collapsed on small PRs)Reviewer's GuideSwitches DockItemModel’s group model ownership from QScopedPointer to QObject parent-child semantics and makes the global model a QPointer, avoiding premature deletion and use-after-free crashes; also guards roleNames() against null globalModel and removes an unnecessary reset() when toggling split mode. Sequence diagram for switching between split and grouped window modessequenceDiagram
actor User
participant DockItemModel
participant GlobalModel as QAbstractItemModel
participant GroupModel as DockGroupModel
User->>DockItemModel: toggleWindowSplit(isWindowSplit)
alt isWindowSplit == true
DockItemModel->>DockItemModel: m_groupModel = nullptr
DockItemModel->>DockItemModel: setSourceModel(m_globalModel)
else isWindowSplit == false
DockItemModel->>DockItemModel: m_groupModel = new DockGroupModel(m_globalModel, DesktopIdRole, this)
DockItemModel->>DockItemModel: setSourceModel(m_groupModel)
end
Sequence diagram for DockItemModel and DockGroupModel destruction via QObject parentingsequenceDiagram
participant QtEngine as QtObjectTree
participant DockItemModel
participant GroupModel as DockGroupModel
QtEngine->>DockItemModel: destroy parent DockItemModel
DockItemModel->>GroupModel: QObject parent-child destruction
destroy GroupModel
destroy DockItemModel
Class diagram for updated DockItemModel ownership and lifetime managementclassDiagram
class QObject
class QAbstractItemModel
class QAbstractProxyModel
class AbstractTaskManagerInterface
class QPointer_QAbstractItemModel_ {
}
class DockGroupModel {
+DockGroupModel(QAbstractItemModel* sourceModel, int groupRole, QObject* parent)
}
class DockItemModel {
+DockItemModel(QAbstractItemModel* globalModel, QObject* parent)
+QHash_int_QByteArray_ roleNames() const
-QPointer_QAbstractItemModel_ m_globalModel
-QAbstractItemModel* m_groupModel
-bool m_split
-bool m_isUpdating
}
DockItemModel --|> QAbstractProxyModel
DockItemModel --|> AbstractTaskManagerInterface
DockItemModel --> QPointer_QAbstractItemModel_ : uses
DockItemModel --> DockGroupModel : creates
DockGroupModel --|> QAbstractItemModel
DockItemModel --|> QObject
DockGroupModel --|> QObject
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![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 found 1 issue, and left some high level feedback:
- When switching repeatedly between split and grouped modes, a new DockGroupModel is allocated each time without deleting or reusing the previous instance, which can leak models until DockItemModel is destroyed; consider deleting the existing m_groupModel (or reusing it) before creating a new one.
- Since m_groupModel is always constructed as a DockGroupModel, it may be clearer and safer to type m_groupModel as a DockGroupModel* rather than QAbstractItemModel* to make ownership and usage more explicit.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- When switching repeatedly between split and grouped modes, a new DockGroupModel is allocated each time without deleting or reusing the previous instance, which can leak models until DockItemModel is destroyed; consider deleting the existing m_groupModel (or reusing it) before creating a new one.
- Since m_groupModel is always constructed as a DockGroupModel, it may be clearer and safer to type m_groupModel as a DockGroupModel* rather than QAbstractItemModel* to make ownership and usage more explicit.
## Individual Comments
### Comment 1
<location> `panels/dock/taskmanager/dockitemmodel.cpp:29-33` </location>
<code_context>
if (isWindowSplit) {
setSourceModel(m_globalModel);
- m_groupModel.reset(nullptr);
} else {
- m_groupModel.reset(new DockGroupModel(m_globalModel, TaskManager::DesktopIdRole));
- setSourceModel(m_groupModel.get());
+ m_groupModel = new DockGroupModel(m_globalModel, TaskManager::DesktopIdRole, this);
+ setSourceModel(m_groupModel);
}
};
</code_context>
<issue_to_address>
**issue (bug_risk):** Avoid leaking DockGroupModel instances when toggling split mode off multiple times.
When `isWindowSplit` is false, a new `DockGroupModel` is created and parented to `this` each time, but the previous `m_groupModel` is never destroyed or reused. Over multiple toggles this will leak `DockGroupModel` instances and their signal/slot connections. Please either delete/reset the existing `m_groupModel` before assigning a new one, reuse a single instance, or switch back to a smart-pointer/RAII approach.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
deepin pr auto review###，我将对这段代码进行详细的审查，并提供改进建议。 1. 代码变更概览这段代码主要对
2. 语法与逻辑审查2.1 语法正确性代码语法上是正确的，符合 C++ 和 Qt 的规范。 2.2 逻辑一致性变更逻辑基本合理，特别是将 3. 代码质量改进建议3.1 内存管理策略变更问题：将
3.2 指针类型混用问题：
3.3 空指针检查问题：虽然
4. 代码性能改进建议4.1 指针访问开销问题：
5. 代码安全改进建议5.1 悬空指针风险问题：虽然
5.2 线程安全问题：如果
6. 具体代码改进建议6.1 头文件改进// 建议在头文件中添加注释说明成员变量的生命周期管理
private:
QPointer<QAbstractItemModel> m_globalModel; // 弱引用，可能在外部被删除
QPointer<QAbstractItemModel> m_groupModel; // 改为 QPointer 以提高安全性
bool m_split;
bool m_isUpdating;6.2 构造函数改进DockItemModel::DockItemModel(QAbstractItemModel *globalModel, QObject *parent)
: QAbstractProxyModel(parent)
, AbstractTaskManagerInterface(this)
, m_globalModel(globalModel)
, m_groupModel(nullptr) // 显式初始化
, m_split(!TaskManagerSettings::instance()->isWindowSplit())
, m_isUpdating(false)
{
// ... 其他代码 ...
}6.3 roleNames() 方法改进QHash<int, QByteArray> DockItemModel::roleNames() const
{
if (!m_globalModel) {
qWarning() << "m_globalModel is null in DockItemModel::roleNames()";
return {};
}
return m_globalModel->roleNames();
}7. 总结这段代码的变更总体上是合理的，特别是将 |
BLumia
approved these changes
Jan 30, 2026
|
[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 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The crash occurred due to improper memory management of DockGroupModel
objects. Previously, m_groupModel was managed using QScopedPointer,
which could lead to premature deletion when the model was still in use.
The issue was fixed by changing to QObject parent-child relationship
management.
tracking
parent-child relationship
parameter, enabling Qt's automatic object tree management
m_globalModel is null
This ensures proper lifetime management where DockGroupModel instances
are automatically deleted when their parent DockItemModel is destroyed,
preventing use-after-free crashes.
Log: Fixed application crash when switching between window grouping
modes
Influence:
fix: 修复DockGroupModel析构问题导致的崩溃
崩溃是由于DockGroupModel对象的内存管理不当引起的。之前使用QScopedPointer
管理m_groupModel，可能导致模型仍在使用时被提前删除。通过改为使用QObject
父子关系管理解决了此问题。
管理
这确保了正确的生命周期管理，当父对象DockItemModel被销毁时，
DockGroupModel实例会自动删除，防止了使用已释放内存导致的崩溃。
Log: 修复了在切换窗口分组模式时应用程序崩溃的问题
Influence:
PMS: BUG-321495
Summary by Sourcery
Improve DockItemModel’s pointer management to prevent crashes when switching between grouped and split window modes.
Bug Fixes:
Enhancements: