fix: prevent crashes and memory leaks during object lifecycle#3168
Open
pengfeixx wants to merge 1 commit intolinuxdeepin:masterfrom
Open
fix: prevent crashes and memory leaks during object lifecycle#3168pengfeixx wants to merge 1 commit intolinuxdeepin:masterfrom
pengfeixx wants to merge 1 commit intolinuxdeepin:masterfrom
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: pengfeixx 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 |
Reviewer's GuideAdds a batch-update mode to suppress intermediate current-object signals during page navigation, hardens DccObject parent handling with null checks, and fixes plugin-related memory leaks and crashes when the plugin manager is tearing down or QML loaders are destroyed. Sequence diagram for batch-updated page navigation and suppressed signalssequenceDiagram
actor User
participant DccManager
participant DccObject
participant DccApp
User->>DccManager: doShowPage(obj, cmd)
activate DccManager
DccManager->>DccManager: m_batchUpdating = true
DccManager->>DccObject: setCurrentObject(nullptr)
activate DccObject
DccObject->>DccApp: instance()
DccApp-->>DccObject: app
DccObject->>DccApp: isBatchUpdating()
DccApp-->>DccObject: true
alt batch updating is true
DccObject-->>DccManager: skip deactive and currentObjectChanged
else batch updating is false
DccObject-->>DccManager: emit deactive
DccObject-->>DccManager: emit currentObjectChanged
end
DccObject-->>DccManager: return
deactivate DccObject
DccManager->>DccManager: update modules and m_currentObjects
DccManager->>DccManager: m_batchUpdating = false
DccManager-->>User: triggeredObjectsChanged, active object updated
deactivate DccManager
Class diagram for DccApp, DccManager, DccObject lifecycle changesclassDiagram
class DccApp {
+void setSidebarWidth(int width)
+void setAnimationMode(AnimationMode mode)
+bool isBatchUpdating() const
}
class DccManager {
+void doShowPage(QPointer_DccObject obj, QString cmd)
+bool isBatchUpdating() const
-QVector_DccObjectPtr m_currentObjects
-QVector_DccObjectPtr m_triggeredObjects
-QHash_QString_QVector_DccObjectPtr m_objMap
-atomic_bool m_batchUpdating
}
class DccObject {
+void setCurrentObject(DccObject *obj)
+DccObject *currentObject()
+void setWeight(quint32 weight)
+QVector_DccObjectPtr getChildren() const
+DccObject *getChild(int childPos) const
-DccObject_Private *p_ptr
}
class DccObject_Private {
-DccObject *q_ptr
-DccObject *m_parent
-QVector_DccObjectPtr m_children
-QObject *m_page
-DccObject *m_currentObject
-quint32 m_weight
+~DccObject_Private()
+const QVector_DccObjectPtr &getChildren() const
+int getIndex() const
+DccObject *getChild(int childPos) const
+void updatePos(DccObject *obj)
}
class PluginManager {
+bool isDeleting() const
+void createModule(QQmlComponent *component)
+void createMain(QQmlComponent *component)
}
class LoadPluginTask {
-PluginManager *m_pManager
-PluginDataHolder *m_data
+void createData()
}
DccApp <|-- DccManager
DccObject o-- DccObject_Private
DccObject_Private --> DccObject : m_parent
DccObject_Private --> DccObject : m_children
DccObject_Private --> DccObject : m_currentObject
PluginManager o-- LoadPluginTask
LoadPluginTask --> PluginManager : m_pManager
class QQmlComponent {
+QVariant property(QString name) const
+void deleteLater()
}
PluginManager --> QQmlComponent : createModule
PluginManager --> QQmlComponent : createMain
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:
- The manual
m_batchUpdatingtoggling inDccManager::doShowPageis easy to break if more early-return paths are added later; consider wrapping it in an RAII helper (e.g. a local guard object) so the flag is always reset on every exit path. - In
DccObject::setCurrentObject,deactiveandcurrentObjectChangedare skipped entirely whenisBatchUpdating()is true; if callers still need a final notification after the batch, consider emitting a consolidated signal once the batch update completes or documenting that these signals are intentionally dropped during batch operations.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The manual `m_batchUpdating` toggling in `DccManager::doShowPage` is easy to break if more early-return paths are added later; consider wrapping it in an RAII helper (e.g. a local guard object) so the flag is always reset on every exit path.
- In `DccObject::setCurrentObject`, `deactive` and `currentObjectChanged` are skipped entirely when `isBatchUpdating()` is true; if callers still need a final notification after the batch, consider emitting a consolidated signal once the batch update completes or documenting that these signals are intentionally dropped during batch operations.
## Individual Comments
### Comment 1
<location path="src/dde-control-center/plugin/dccobject.cpp" line_range="446-449" />
<code_context>
- if (p_ptr->m_currentObject) {
- Q_EMIT p_ptr->m_currentObject->deactive();
- }
+ DccObject *oldObject = p_ptr->m_currentObject;
p_ptr->m_currentObject = obj;
- Q_EMIT currentObjectChanged(p_ptr->m_currentObject);
+
+ DccApp *app = DccApp::instance();
</code_context>
<issue_to_address>
**issue (bug_risk):** Reordered deactivation vs. current object update may introduce behavior changes in slots.
Previously, `deactive()` was emitted before updating `m_currentObject`, so slots handling `deactive()` and then calling `currentObject()` would still see the old object. With the new ordering (update `m_currentObject` first, then emit `deactive(oldObject)`), those same slots will now see the new object, which is a behavior change that can affect existing plugin code. If the goal is just to suppress signals during batch updates, you could keep the original ordering and gate both signals with `isBatchUpdating()` instead.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+446
to
+449
| DccObject *oldObject = p_ptr->m_currentObject; | ||
| p_ptr->m_currentObject = obj; | ||
| Q_EMIT currentObjectChanged(p_ptr->m_currentObject); | ||
|
|
||
| DccApp *app = DccApp::instance(); |
There was a problem hiding this comment.
issue (bug_risk): Reordered deactivation vs. current object update may introduce behavior changes in slots.
Previously, deactive() was emitted before updating m_currentObject, so slots handling deactive() and then calling currentObject() would still see the old object. With the new ordering (update m_currentObject first, then emit deactive(oldObject)), those same slots will now see the new object, which is a behavior change that can affect existing plugin code. If the goal is just to suppress signals during batch updates, you could keep the original ordering and gate both signals with isBatchUpdating() instead.
Add batch update flag to suppress intermediate signals during page navigation, add null checks for parent private pointer in DccObject, and fix component memory leaks when plugin manager is deleting. 添加批量更新标志以抑制页面导航期间的中间信号发射， 增加 DccObject 父对象私有指针的空检查，修复插件管理器 删除时的组件内存泄漏问题。 Log: 修复对象生命周期中的崩溃和内存泄漏问题 PMS: BUG-335919 Influence: 修复控制中心页面切换过程中可能出现的崩溃问题， 修复插件卸载时的内存泄漏，提升稳定性。
18202781743
reviewed
Apr 15, 2026
| delete dataObj; | ||
| if (soObj) | ||
| delete soObj; | ||
| return; |
Contributor
There was a problem hiding this comment.
直接return，不走 这个正常么， Q_EMIT m_pManager->updatePluginStatus(m_data, DataEnd, ": create data finished. elapsed time :" + QString::number(timer.elapsed()));
| Q_EMIT currentObjectChanged(p_ptr->m_currentObject); | ||
|
|
||
| DccApp *app = DccApp::instance(); | ||
| if (!app || !app->isBatchUpdating()) { |
Contributor
There was a problem hiding this comment.
看下ai的评论，另外app为空不需要执行了吧，
| @@ -442,11 +443,16 @@ DccObject *DccObject::currentObject() | |||
| void DccObject::setCurrentObject(DccObject *obj) | |||
Contributor
There was a problem hiding this comment.
这应该是在主线程里被触发的吧，是线性的吧，会存在多个交叉被setCurrentObject么，
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
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.
Add batch update flag to suppress intermediate signals during page navigation, add null checks for parent private pointer in DccObject, and fix component memory leaks when plugin manager is deleting.
添加批量更新标志以抑制页面导航期间的中间信号发射，
增加 DccObject 父对象私有指针的空检查，修复插件管理器
删除时的组件内存泄漏问题。
Log: 修复对象生命周期中的崩溃和内存泄漏问题
PMS: BUG-335919
Influence: 修复控制中心页面切换过程中可能出现的崩溃问题，
修复插件卸载时的内存泄漏，提升稳定性。
Summary by Sourcery
Improve object lifecycle handling to avoid crashes and memory leaks during page navigation and plugin teardown.
Bug Fixes:
Enhancements: