fix: Change the QML creation method to asynchronous creation.#3160
Open
caixr23 wants to merge 5 commits intolinuxdeepin:dcc-coredumpfrom
Open
fix: Change the QML creation method to asynchronous creation.#3160caixr23 wants to merge 5 commits intolinuxdeepin:dcc-coredumpfrom
caixr23 wants to merge 5 commits intolinuxdeepin:dcc-coredumpfrom
Conversation
The image provider has been refactored to eliminate mutex locks and implement fully asynchronous processing. The main changes include: 1. Removed QMutex and QMutexLocker usage entirely, replacing thread synchronization with thread-safe Qt signals/slots 2. Restructured CacheImageResponse to be a pure response container without loading logic 3. Created ImageTask as a QRunnable that loads images asynchronously on QThreadPool::globalInstance() 4. Implemented a pending response tracking system using QMultiMap to handle multiple requests for the same image 5. Added proper thread affinity management with moveToThread() for responses 6. Simplified cache key generation with a dedicated makeCacheKey() method The refactoring addresses performance issues by removing contention on mutex locks during image loading. Multiple requests for the same image now share the same loading task instead of creating duplicate tasks. The response objects are properly managed with QPointer to handle potential deletion during asynchronous operations. Influence: 1. Test image loading with different sizes and formats 2. Verify concurrent requests for the same image don't create duplicate loading tasks 3. Test image provider with invalid image paths to ensure proper error handling 4. Verify memory management and proper cleanup of response objects 5. Test performance under high load with multiple simultaneous image requests 6. Ensure thread safety without race conditions in the new async architecture refactor: 移除图标处理中的互斥锁，使用异步处理 图像提供器已重构，消除了互斥锁并实现了完全异步处理。主要变更包括： 1. 完全移除了QMutex和QMutexLocker的使用，用线程安全的Qt信号/槽替代线程 同步 2. 重构CacheImageResponse为纯响应容器，不包含加载逻辑 3. 创建ImageTask作为QRunnable，在QThreadPool::globalInstance()上异步加载 图像 4. 实现了使用QMultiMap的待处理响应跟踪系统，处理同一图像的多个请求 5. 通过moveToThread()为响应添加了适当的线程亲和性管理 6. 使用专用的makeCacheKey()方法简化了缓存键生成 此次重构解决了图像加载期间互斥锁争用导致的性能问题。现在对同一图像的多个 请求共享相同的加载任务，而不是创建重复任务。响应对象通过QPointer进行适当 管理，以处理异步操作期间可能的删除操作。 Influence: 1. 测试不同尺寸和格式的图像加载 2. 验证对同一图像的并发请求不会创建重复的加载任务 3. 测试图像提供器使用无效图像路径时的错误处理 4. 验证内存管理和响应对象的正确清理 5. 测试高负载下多个同时图像请求的性能 6. 确保新异步架构中的线程安全，避免竞态条件
1. Extracted plugin factory preparation logic from LoadPluginTask::doLoadSo() into new method preparePluginFactory() 2. Modified LoadPluginTask::doLoadSo() to LoadPluginTask::createData() that uses pre-prepared factory 3. Added factory pointer to PluginData structure to store prepared factory instance 4. Moved factory preparation to occur before loadModule() in plugin loading sequence 5. Simplified createData() method to focus only on object creation from factory 6. Added proper cleanup of factory pointer in PluginManager destructor This refactoring separates factory preparation from object creation, allowing factory initialization to happen earlier in the plugin loading process. The change improves code organization by separating concerns: factory loading/validation vs. object instantiation. This prepares for potential optimizations where factory preparation could be done in parallel or at different stages of application startup. Influence: 1. Verify plugin loading still works correctly for all control center modules 2. Test plugin loading with both valid and invalid .so files 3. Verify error handling when factory preparation fails 4. Test memory management and cleanup of factory instances 5. Ensure plugin status updates are still emitted correctly 6. Verify thread safety during factory preparation and object creation 7. Test with plugins that have parent/child relationships in their created objects refactor: 将插件工厂准备逻辑前移到模块加载之前执行 1. 从 LoadPluginTask::doLoadSo() 中提取插件工厂准备逻辑到新的 preparePluginFactory() 方法 2. 将 LoadPluginTask::doLoadSo() 重命名为 LoadPluginTask::createData()， 使用预先准备的工厂 3. 在 PluginData 结构中添加工厂指针以存储准备好的工厂实例 4. 将工厂准备移动到插件加载序列中的 loadModule() 之前执行 5. 简化 createData() 方法，专注于从工厂创建对象 6. 在 PluginManager 析构函数中添加工厂指针的适当清理 此次重构将工厂准备与对象创建分离，允许工厂初始化在插件加载过程的更早阶段 进行。这一改动通过分离关注点（工厂加载/验证 vs. 对象实例化）改善了代码组 织。这为潜在的优化做好了准备，工厂准备可以并行进行或在应用程序启动的不同 阶段执行。 Influence: 1. 验证所有控制中心模块的插件加载是否仍然正常工作 2. 测试使用有效和无效 .so 文件的插件加载 3. 验证工厂准备失败时的错误处理 4. 测试工厂实例的内存管理和清理 5. 确保插件状态更新仍然正确发出 6. 验证工厂准备和对象创建期间的线程安全性 7. 测试具有父子关系的插件对象的创建
Changed plugin loading logic to ensure all plugins complete their module phase before any plugin starts the data phase. Previously, when a plugin reached ModuleEnd status, it would immediately proceed to LoadPluginTask for data loading. Now, the system waits until all plugins have finished their module phase (ModuleEnd, ModuleErr, or PluginEnd status) before starting data loading for any plugin. Key changes: 1. Added m_modulePhaseFinished flag to track module phase completion 2. Added allModulesFinished() method to check if all plugins have completed module phase 3. Added loadPluginData() method extracted from original loadPlugin() logic 4. Added onModulePhaseFinished() slot to handle transition to data phase 5. Modified loadPlugin() to check module phase completion before proceeding to data loading 6. Connected modulePhaseFinished signal to onModulePhaseFinished slot This ensures that plugins don't start their data loading (createData()) until all plugins have completed their module initialization, preventing potential race conditions and ensuring proper synchronization between plugins. Log: Fixed plugin loading synchronization issue Influence: 1. Test plugin loading with multiple plugins to ensure all complete module phase before data phase starts 2. Verify that plugins with ModuleEnd status wait for others before proceeding 3. Test edge cases where some plugins may have ModuleErr or PluginEnd status 4. Verify that the m_modulePhaseFinished flag is properly reset when needed 5. Test plugin loading performance to ensure no regression 6. Verify that hidden plugins (not visibleToApp) still follow the synchronization logic fix: 同步插件数据阶段加载 修改插件加载逻辑，确保所有插件完成模块阶段后再开始数据阶段。之前，当插件 达到 ModuleEnd 状态时，会立即进入 LoadPluginTask 进行数据加载。现在，系 统会等待所有插件完成模块阶段（ModuleEnd、ModuleErr 或 PluginEnd 状态）后 才开始任何插件的数据加载。 主要变更： 1. 添加 m_modulePhaseFinished 标志来跟踪模块阶段完成状态 2. 添加 allModulesFinished() 方法来检查所有插件是否已完成模块阶段 3. 添加 loadPluginData() 方法，从原始 loadPlugin() 逻辑中提取 4. 添加 onModulePhaseFinished() 槽函数来处理向数据阶段的过渡 5. 修改 loadPlugin() 以在进入数据加载前检查模块阶段完成状态 6. 连接 modulePhaseFinished 信号到 onModulePhaseFinished 槽 这确保了插件在所有插件完成模块初始化之前不会开始数据加载 （createData()），防止潜在的竞争条件并确保插件之间的正确同步。 Log: 修复插件加载同步问题 Influence: 1. 测试多插件加载，确保所有插件在数据阶段开始前完成模块阶段 2. 验证具有 ModuleEnd 状态的插件在继续之前会等待其他插件 3. 测试某些插件可能具有 ModuleErr 或 PluginEnd 状态的边缘情况 4. 验证 m_modulePhaseFinished 标志在需要时正确重置 5. 测试插件加载性能，确保没有回归问题 6. 验证隐藏插件（不可见）仍然遵循同步逻辑
1. Added QQmlParserStatus interface to DccObject to track QML component lifecycle 2. Introduced m_componentComplete flag to track when component construction is finished 3. Modified setIcon() to delay icon source resolution until componentComplete() is called 4. Added updateIconSource() private method to handle icon URL resolution logic 5. Added componentComplete() override to set completion flag and trigger icon source update 6. Added classBegin() override as required by QQmlParserStatus interface 7. Made DccObject inherit from QQmlParserStatus and added Q_INTERFACES macro 8. Added friend declaration for DccModel class to access private members This fix addresses an issue where icon source resolution was attempted during QML component construction phase when the QQmlContext might not be fully initialized. By deferring the URL resolution until componentComplete(), we ensure the context is properly set up, preventing potential crashes or incorrect icon paths. Log: Fixed icon loading issues during control center module initialization Influence: 1. Test icon loading for all control center modules 2. Verify icons appear correctly after QML component initialization 3. Test dynamic icon changes after component completion 4. Verify no crashes during control center startup 5. Test icon resolution with both relative and absolute paths 6. Verify icon updates when setIcon() is called after component completion fix: 延迟图标源解析直到组件完成 1. 为 DccObject 添加 QQmlParserStatus 接口以跟踪 QML 组件生命周期 2. 引入 m_componentComplete 标志来跟踪组件构造何时完成 3. 修改 setIcon() 方法，将图标源解析延迟到 componentComplete() 被调用时 4. 添加 updateIconSource() 私有方法来处理图标 URL 解析逻辑 5. 添加 componentComplete() 重写以设置完成标志并触发图标源更新 6. 添加 classBegin() 重写以满足 QQmlParserStatus 接口要求 7. 使 DccObject 继承自 QQmlParserStatus 并添加 Q_INTERFACES 宏 8. 为 DccModel 类添加友元声明以访问私有成员 此修复解决了在 QML 组件构造阶段尝试解析图标源时 QQmlContext 可能未完全初 始化的问题。通过将 URL 解析延迟到 componentComplete()，我们确保上下文已 正确设置，防止潜在的崩溃或错误的图标路径。 Log: 修复控制中心模块初始化期间的图标加载问题 Influence: 1. 测试所有控制中心模块的图标加载 2. 验证 QML 组件初始化后图标是否正确显示 3. 测试组件完成后的动态图标更改 4. 验证控制中心启动期间无崩溃 5. 测试相对路径和绝对路径的图标解析 6. 验证组件完成后调用 setIcon() 时的图标更新
Change the QML creation method to asynchronous creation.
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: caixr23 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 GuideRefactors plugin loading and QML object creation to be fully asynchronous, restructures plugin lifecycle steps (SO loading, data creation, main object creation), and tightens ownership/parenting and visibility-based loading while simplifying thread pool and manager utilities. Sequence diagram for asynchronous plugin loading and QML main object creationsequenceDiagram
participant DccManager
participant PluginManager
participant PluginData
participant DccModule as DccObject_module
participant QThreadPool
participant LoadPluginTask
participant DccFactory
participant QQmlComponent
participant QQmlContext
participant DccIncubator
participant MainObj as DccObject_main
DccManager->>PluginManager: loadPlugin(plugin)
PluginManager->>PluginData: check status flags
alt module finished and visible
PluginManager->>DccModule: isVisibleToApp()
alt visible
PluginManager->>PluginManager: doLoadSo(plugin,this)
PluginManager->>PluginManager: doCreateDccObject(plugin,this)
PluginManager->>QThreadPool: start(new LoadPluginTask(plugin,this))
QThreadPool-->>LoadPluginTask: run() on worker thread
LoadPluginTask->>PluginData: set thread
LoadPluginTask->>PluginManager: doCreateData(plugin,this)
PluginManager->>DccFactory: create()
DccFactory-->>PluginManager: data QObject
PluginManager->>PluginData: store data and moveToThread(main)
LoadPluginTask->>PluginData: clear thread
LoadPluginTask-->>PluginManager: finished
PluginManager->>PluginManager: loadMain(plugin)
PluginManager->>QQmlComponent: createMain(component)
QQmlComponent-->>PluginManager: status Ready
PluginManager->>QQmlContext: new QQmlContext(engine,engine)
PluginManager->>QQmlContext: setContextProperties(dccData,dccModule)
PluginManager->>DccIncubator: new DccIncubator(this,plugin,component,context)
PluginManager->>QQmlComponent: create(*incubator,context,nullptr)
QQmlComponent-->>DccIncubator: begin asynchronous incubation
DccIncubator-->>DccIncubator: statusChanged(Ready)
DccIncubator->>MainObj: object() created
DccIncubator->>QQmlContext: setParent(object)
DccIncubator->>MainObj: setParent(plugin.module or rootModule)
DccIncubator->>PluginData: set mainObj
DccIncubator->>PluginManager: updatePluginStatus(MainObjEnd)
DccIncubator->>QQmlComponent: deleteLater()
DccIncubator-->>DccIncubator: delete this
PluginManager->>PluginManager: addMainObject(plugin)
PluginManager->>DccManager: addObject(mainObj)
PluginManager->>DccManager: addObject(soObj if exists)
else not visible
PluginManager->>PluginManager: updatePluginStatus(PluginEnd)
PluginManager->>DccModule: connect visibleToAppChanged
end
else no module
PluginManager->>PluginManager: doLoadSo(plugin,this)
PluginManager->>PluginManager: doCreateDccObject(plugin,this)
PluginManager->>QThreadPool: start(new LoadPluginTask(plugin,this))
QThreadPool-->>LoadPluginTask: run() and doCreateData(plugin,this)
end
note over PluginManager,QThreadPool: cancelLoad() now does nothing, global thread pool is always used
Class diagram for updated plugin loading and Dcc object managementclassDiagram
class PluginManager {
- DccManager *m_manager
- QList~PluginData*~ m_plugins
- DccObject *m_rootModule
- bool m_isDeleting
- bool m_modulePhaseFinished
- QQmlEngine *m_engine
+ PluginManager(DccManager *parent)
+ QThreadPool *threadPool()
+ void loadPlugin(PluginData *plugin)
+ void loadMetaData(PluginData *plugin)
+ void createMain(QQmlComponent *component)
+ void addMainObject(PluginData *plugin)
+ void cancelLoad()
+ bool allModulesFinished() const
+ bool isDeleting() const
+ QThread *thread()
+ Q_SIGNALS void updatePluginStatus(PluginData *plugin, uint status, QString msg)
+ Q_SIGNALS void addObject(DccObject *obj)
}
class PluginData {
+ QString name
+ QString path
+ uint type
+ uint status
+ QThread *thread
+ DccFactory *factory
+ QObject *data
+ DccObject *module
+ DccObject *mainObj
+ DccObject *soObj
+ uint version()
}
class LoadPluginTask {
- PluginData *m_data
- PluginManager *m_pManager
+ LoadPluginTask(PluginData *data, PluginManager *pManager)
+ void run()
}
class DccIncubator {
- PluginManager *m_pManager
- PluginData *m_data
- QQmlComponent *m_component
- QQmlContext *m_context
+ DccIncubator(PluginManager *pManager, PluginData *data, QQmlComponent *component, QQmlContext *context)
+ void setInitialState(QObject *object)
+ void statusChanged(Status status)
}
class DccObject {
- Private *p_ptr
+ void classBegin()
+ void componentComplete()
+ QString parentName() const
+ QString name() const
+ const QVector~DccObject*~ &getChildren() const
+ Q_SIGNALS void addObject(DccObject *obj)
+ Q_SIGNALS void removeObject(DccObject *obj)
}
class DccObject_Private {
+ DccObject *q_ptr
+ QVector~DccObject*~ m_children
+ QVector~DccObject*~ m_objects
+ QObjectList m_data
+ QPointer~QQmlComponent~ m_page
+ QPointer~QQuickItem~ m_parentItem
+ DccObject_Private *m_pParent
+ QString m_parentName
+ QString m_displayName
+ QString m_name
+ ~Private()
+ void addObject(DccObject *child)
+ void removeObject(DccObject *child)
+ void clearObject()
+ const QVector~DccObject*~ &getChildren() const
}
class DccManager {
- PluginManager *m_plugins
- DccObject *m_root
- DccObject *m_hideObjects
- DccObject *m_noAddObjects
- DccObject *m_noParentObjects
- QSet~QString~ m_hideModule
- QTimer *m_showTimer
- QString m_showUrl
- DccObject *m_activeObject
+ DccManager(QObject *parent)
+ void initConfig()
+ void tryShow()
+ void clearData()
+ inline const QSet~QString~ &hideModule() const
+ static bool isMatchByName(const QString &url, const QString &name)
+ static bool isMatch(const QString &url, const DccObject *obj)
+ static bool isEqualByName(const QString &url, const QString &name)
+ static bool isEqual(const QString &url, const DccObject *obj)
+ static bool containsByName(const QSet~QString~ &urls, const QString &name)
+ static bool contains(const QSet~QString~ &urls, const DccObject *obj)
+ Q_SLOTS void addObject(DccObject *obj)
}
class DccRepeaterPrivate {
+ QQmlInstanceModel *model
+ void createItems(const QVector~bool~ &deletables)
}
class DccRepeater {
- DccRepeaterPrivate *d_ptr
- DccObject::Private *p_ptr
+ void createdItem(int index, QObject *item)
+ Q_SIGNALS void objAdded(int index, QObject *item)
}
PluginManager ..> PluginData : manages
PluginManager ..> LoadPluginTask : creates
PluginManager ..> DccIncubator : creates
PluginManager ..> DccManager : uses
PluginManager ..> DccObject : uses as module and mainObj
LoadPluginTask ..> PluginData : operates on
LoadPluginTask ..> PluginManager : calls helpers
DccIncubator --|> QQmlIncubator
DccIncubator ..> PluginManager : updatePluginStatus
DccIncubator ..> PluginData : set mainObj
DccIncubator ..> DccObject : main UI object
DccObject *-- DccObject_Private : p_ptr
DccObject_Private o--> DccObject_Private : m_pParent
DccObject_Private o--> DccObject : m_children
DccObject_Private o--> DccObject : m_objects
DccManager o--> PluginManager : m_plugins
DccManager o--> DccObject : m_root
DccRepeater *-- DccRepeaterPrivate : d_ptr
DccRepeater ..> DccObject : created items
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 3 issues, and left some high level feedback:
- In
doLoadSotheQPluginLoaderis a local variable and goes out of scope afterplugin->factoryis set, which means the plugin may be unloaded while its factory instance is still used; consider keeping a persistentQPluginLoader(e.g., as part ofPluginData) for the plugin's lifetime. - In
DccRepeater::createdItemyou calld->model->object(index);again after the item is reported as created, which can force a second creation or unnecessary work; if you only need the already-createddccObj, this extra call is likely redundant and could be removed.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `doLoadSo` the `QPluginLoader` is a local variable and goes out of scope after `plugin->factory` is set, which means the plugin may be unloaded while its factory instance is still used; consider keeping a persistent `QPluginLoader` (e.g., as part of `PluginData`) for the plugin's lifetime.
- In `DccRepeater::createdItem` you call `d->model->object(index);` again after the item is reported as created, which can force a second creation or unnecessary work; if you only need the already-created `dccObj`, this extra call is likely redundant and could be removed.
## Individual Comments
### Comment 1
<location path="src/dde-control-center/pluginmanager.cpp" line_range="245" />
<code_context>
+ QObject *obj = object();
+ if (obj) {
+ m_context->setParent(obj);
+ obj->setParent(m_data->module ? m_data->module : nullptr);
+ m_data->mainObj = qobject_cast<DccObject *>(obj);
+ Q_EMIT m_pManager->updatePluginStatus(m_data, MainObjEnd, ": create main finished");
</code_context>
<issue_to_address>
**issue (bug_risk):** Main object loses fallback parent when module is null
In the old `createMain` path, the parent was `plugin->module ? plugin->module : m_rootModule`. Here, when `m_data->module` is null, you set the parent to `nullptr`, so these main objects are no longer attached under `m_rootModule` and their ownership/lifetime may change. If that isn’t an intentional behavior change, please restore the `m_rootModule` fallback.
</issue_to_address>
### Comment 2
<location path="src/dde-control-center/pluginmanager.cpp" line_range="438-440" />
<code_context>
QThreadPool *PluginManager::threadPool()
{
- if (!m_threadPool) {
- m_threadPool = new QThreadPool(this);
- }
- return m_threadPool;
+ return QThreadPool::globalInstance();
}
</code_context>
<issue_to_address>
**issue (bug_risk):** Switching to the global thread pool makes cancelLoad() effectively inert
Previously `PluginManager` had its own `QThreadPool`, and `cancelLoad()` would `clear()` and `waitForDone()` to stop outstanding work during teardown. With `threadPool()` now returning `QThreadPool::globalInstance()`, `cancelLoad()` effectively does nothing, so any caller assuming it cancels plugin tasks will be broken and may see teardown races. Consider either keeping a dedicated pool per `PluginManager` to preserve cancellation semantics, or introducing an alternative coordination mechanism (e.g., stronger `m_isDeleting` checks in tasks plus queued-connection barriers).
</issue_to_address>
### Comment 3
<location path="src/dde-control-center/plugin/dccrepeater.cpp" line_range="189" />
<code_context>
void DccRepeater::createdItem(int index, QObject *item)
</code_context>
<issue_to_address>
**suggestion (performance):** Calling model->object(index) in createdItem() may be redundant and expensive
In `createdItem`, after casting to `DccObject`, `d->model->object(index)` is called but its result is unused. Unless this is intentionally forcing instantiation and the model doesn’t already guarantee that at this point, this call seems unnecessary and could add overhead or unexpected side effects. Please confirm whether it’s needed; if not, it can be removed.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| QObject *obj = object(); | ||
| if (obj) { | ||
| m_context->setParent(obj); | ||
| obj->setParent(m_data->module ? m_data->module : nullptr); |
There was a problem hiding this comment.
issue (bug_risk): Main object loses fallback parent when module is null
In the old createMain path, the parent was plugin->module ? plugin->module : m_rootModule. Here, when m_data->module is null, you set the parent to nullptr, so these main objects are no longer attached under m_rootModule and their ownership/lifetime may change. If that isn’t an intentional behavior change, please restore the m_rootModule fallback.
Comment on lines
438
to
+440
| QThreadPool *PluginManager::threadPool() | ||
| { | ||
| if (!m_threadPool) { | ||
| m_threadPool = new QThreadPool(this); | ||
| } | ||
| return m_threadPool; | ||
| return QThreadPool::globalInstance(); |
There was a problem hiding this comment.
issue (bug_risk): Switching to the global thread pool makes cancelLoad() effectively inert
Previously PluginManager had its own QThreadPool, and cancelLoad() would clear() and waitForDone() to stop outstanding work during teardown. With threadPool() now returning QThreadPool::globalInstance(), cancelLoad() effectively does nothing, so any caller assuming it cancels plugin tasks will be broken and may see teardown races. Consider either keeping a dedicated pool per PluginManager to preserve cancellation semantics, or introducing an alternative coordination mechanism (e.g., stronger m_isDeleting checks in tasks plus queued-connection barriers).
| DccObject *dccObj = qmlobject_cast<DccObject *>(item); | ||
| if (dccObj) { | ||
| Q_D(const DccRepeater); | ||
| d->model->object(index); |
There was a problem hiding this comment.
suggestion (performance): Calling model->object(index) in createdItem() may be redundant and expensive
In createdItem, after casting to DccObject, d->model->object(index) is called but its result is unused. Unless this is intentionally forcing instantiation and the model doesn’t already guarantee that at this point, this call seems unnecessary and could add overhead or unexpected side effects. Please confirm whether it’s needed; if not, it can be removed.
18202781743
reviewed
Apr 8, 2026
| inline uint version() { return type & T_V_MASK; } | ||
| }; | ||
|
|
||
| static void doLoadSo(PluginData *plugin, PluginManager *pManager) |
Contributor
There was a problem hiding this comment.
把它当做静态函数有啥好处么？除了多参数，传递麻烦外，没感觉有啥好处呀，
| break; | ||
| } | ||
| m_component->deleteLater(); | ||
| delete this; |
Contributor
There was a problem hiding this comment.
成员函数里delete自己？一般生命周期是外部管理吧，
mhduiy
reviewed
Apr 8, 2026
| break; | ||
| } | ||
| m_component->deleteLater(); | ||
| delete this; |
Contributor
There was a problem hiding this comment.
由谁创建就谁去释放把. 提供一个完成的信号出去或者通过其他的方式
18202781743
reviewed
Apr 8, 2026
| if (plugin->data) { | ||
| plugin->data->moveToThread(pManager->thread()); | ||
| } | ||
| if (plugin->factory) { |
Contributor
There was a problem hiding this comment.
plugin->factory 就是在主线程中的吧，跟pManager在一个线程里，
|
|
||
| protected: | ||
| // 当对象孵化（创建）完成时调用 | ||
| void setInitialState(QObject *object) override |
There was a problem hiding this comment.
Pull request overview
This PR switches plugin QML loading to asynchronous creation (via QQmlIncubator) and updates plugin/module lifecycle handling to reduce UI blocking and improve robustness during plugin loading and navigation.
Changes:
- Migrate main QML component creation to asynchronous incubation and adjust plugin load sequencing.
- Rework plugin loading to use
QThreadPool::globalInstance()and split SO/data/object creation into helpers. - Refine Dcc object hierarchy bookkeeping and update hidden-module matching / startup navigation behavior.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| src/dde-control-center/pluginmanager.h | Removes per-manager thread pool member in favor of global thread pool usage. |
| src/dde-control-center/pluginmanager.cpp | Introduces async QML incubation and refactors plugin SO/data loading and lifecycle transitions. |
| src/dde-control-center/plugin/dccrepeater.cpp | Changes delegate item creation incubation mode and adjusts created-item handling. |
| src/dde-control-center/plugin/dccobject.cpp | Adjusts ownership/bookkeeping for objects added via data, and alters destruction behavior. |
| src/dde-control-center/plugin/dccobject_p.h | Adds private-parent tracking for m_objects. |
| src/dde-control-center/dccmanager.h | Promotes URL/name matching helpers to public static APIs. |
| src/dde-control-center/dccmanager.cpp | Updates addObject connection type, hidden-module matching, and root-page fallback behavior in tryShow(). |
Comments suppressed due to low confidence (1)
src/dde-control-center/dccmanager.cpp:789
tryShow()now shows the root page wheneverm_showUrlis empty andm_showTimerexists. Sincem_showTimeris created/started in the constructor, this can override an already-active page when the timer fires (previously gated by!m_activeObject). Consider restoring the!m_activeObjectcheck (or otherwise ensuring the timer only triggers initial display when nothing is active).
void DccManager::tryShow()
{
if (m_showUrl.isEmpty() && m_showTimer) {
clearShowParam();
showPage(m_root, QString());
return;
}
Comment on lines
804
to
807
| } | ||
|
|
||
| void PluginManager::cancelLoad() | ||
| { | ||
| if (m_threadPool) { | ||
| m_threadPool->clear(); | ||
| // 等待所有正在运行的任务完成,避免析构时的竞态条件 | ||
| m_threadPool->waitForDone(); | ||
| for (auto &&plugin : m_plugins) { | ||
| if (plugin->thread) { | ||
| qCWarning(dccLog()) << plugin->name << ": status" << QString::number(plugin->status, 16) << "thread exit timeout"; | ||
| } | ||
| } | ||
| qCWarning(dccLog()) << "delete threadPool"; | ||
| delete m_threadPool; | ||
| qCWarning(dccLog()) << "delete threadPool finish"; | ||
| m_threadPool = nullptr; | ||
| } | ||
| } | ||
| void PluginManager::cancelLoad() { } | ||
|
|
There was a problem hiding this comment.
cancelLoad() is now a no-op, but plugins are still loaded via QThreadPool::globalInstance() (LoadPluginTask captures raw PluginData* and writes into it). PluginManager::~PluginManager() deletes PluginData immediately after calling cancelLoad(), so any in-flight tasks can dereference freed memory (UAF) and/or emit signals on a destructed manager. Please restore deterministic cancellation/cleanup (e.g., keep a dedicated QThreadPool per PluginManager and waitForDone(), or track scheduled runnables and block destruction until they finish, or make the task only hold safe weak references and avoid touching PluginData after deletion).
| QObject *obj = object(); | ||
| if (obj) { | ||
| m_context->setParent(obj); | ||
| obj->setParent(m_data->module ? m_data->module : nullptr); |
There was a problem hiding this comment.
In async incubation Ready path, obj->setParent(m_data->module ? m_data->module : nullptr) drops the fallback parent used previously (m_rootModule). For plugins without a module, this leaves the created main object unparented (lifetime leaks) and it may never be owned/cleaned up with the object tree. Please parent to m_data->module when present, otherwise fall back to the same root owner as before (e.g., m_rootModule or another well-defined QObject owner).
Suggested change
| obj->setParent(m_data->module ? m_data->module : nullptr); | |
| obj->setParent(m_data->module ? m_data->module : m_pManager); |
Comment on lines
+253
to
262
| case Error: { | ||
| Q_EMIT m_pManager->updatePluginStatus(m_data, MainObjErr | MainObjEnd, " component create main object error:" + m_component->errorString()); | ||
| } break; | ||
| default: | ||
| return; | ||
| break; | ||
| } | ||
| m_component->deleteLater(); | ||
| delete this; | ||
| } |
There was a problem hiding this comment.
DccIncubator keeps raw pointers to PluginManager and PluginData and deletes itself from statusChanged(). With asynchronous incubation, statusChanged() can run after beginDelete() / ~PluginManager() has started and after PluginData has been freed, leading to UAF when emitting updatePluginStatus or writing m_data->mainObj. Consider guarding with QPointer<PluginManager> (and null-checking), ensuring PluginData stays alive until incubation finishes, and cleaning up m_context on the Error path as well (currently it stays parented to the engine).
Comment on lines
+466
to
+476
| doLoadSo(plugin, this); | ||
| doCreateDccObject(plugin, this); | ||
| threadPool()->start(new LoadPluginTask(plugin, this)); | ||
| } else { | ||
| connect(plugin->module, &DccObject::visibleToAppChanged, this, &PluginManager::onVisibleToAppChanged); | ||
| Q_EMIT updatePluginStatus(plugin, PluginEnd, QString()); | ||
| } | ||
| } else { | ||
| doLoadSo(plugin, this); | ||
| doCreateDccObject(plugin, this); | ||
| threadPool()->start(new LoadPluginTask(plugin, this)); |
There was a problem hiding this comment.
loadPlugin() no longer emits modulePhaseFinished() / uses allModulesFinished() gating, but modulePhaseFinished signal, m_modulePhaseFinished, and onModulePhaseFinished() still remain wired up. If the two-phase module/data lifecycle is no longer used, please remove the now-dead signal/slot/state (and the unused loadPluginData() path) to avoid misleading lifecycle behavior; if it is still required, reintroduce the emission so the second phase can run deterministically.
Suggested change
| doLoadSo(plugin, this); | |
| doCreateDccObject(plugin, this); | |
| threadPool()->start(new LoadPluginTask(plugin, this)); | |
| } else { | |
| connect(plugin->module, &DccObject::visibleToAppChanged, this, &PluginManager::onVisibleToAppChanged); | |
| Q_EMIT updatePluginStatus(plugin, PluginEnd, QString()); | |
| } | |
| } else { | |
| doLoadSo(plugin, this); | |
| doCreateDccObject(plugin, this); | |
| threadPool()->start(new LoadPluginTask(plugin, this)); | |
| Q_EMIT modulePhaseFinished(); | |
| } else { | |
| connect(plugin->module, &DccObject::visibleToAppChanged, this, &PluginManager::onVisibleToAppChanged); | |
| Q_EMIT updatePluginStatus(plugin, PluginEnd, QString()); | |
| } | |
| } else { | |
| Q_EMIT modulePhaseFinished(); |
Comment on lines
+188
to
+189
| Q_D(const DccRepeater); | ||
| d->model->object(index); |
There was a problem hiding this comment.
createdItem() now calls d->model->object(index) after the item has already been created/emitted by the model. QQmlInstanceModel::object() increments the model's reference/lease on the created object and requires a matching release(). Since this extra acquisition is never released, it can leak items or prevent them from being destroyed. Please remove this call (or ensure it is paired with a release() if it is intentionally acquiring a second reference).
Suggested change
| Q_D(const DccRepeater); | |
| d->model->object(index); |
| if (m_pParent) { | ||
| m_pParent->removeObject(q_ptr); | ||
| } | ||
| clearObject(); |
There was a problem hiding this comment.
DccObject::Private::~Private() no longer detaches and deleteLater()s m_page. Since setPage() parents a no-parent QQmlComponent to the DccObject, this regression reintroduces synchronous deletion of QML-owned objects during page transitions, which the removed code explicitly avoided to prevent JS/GC crashes. Please restore the safe deleteLater() cleanup (or an equivalent strategy) for m_page when it is owned by the DccObject.
Suggested change
| clearObject(); | |
| clearObject(); | |
| if (m_page && m_page->parent() == q_ptr) { | |
| // Detach from parent to prevent ~QObject() from doing a synchronous | |
| // delete, then defer destruction so QML's GC can safely process any | |
| // remaining QObjectWrapper references to this page component. | |
| m_page->setParent(nullptr); | |
| m_page->deleteLater(); | |
| } |
Comment on lines
+202
to
+204
| if (child) { | ||
| child->p_ptr->m_pParent = nullptr; | ||
| m_objects.removeOne(child); |
There was a problem hiding this comment.
removeObject() now emits removeObject(child) even when child was not present in m_objects (and also clears child->m_pParent unconditionally). This can cause duplicate/unbalanced add/remove notifications to observers like DccManager, and make bookkeeping harder to reason about. Consider keeping the previous guard (if (removeOne)) and only emitting when an actual removal occurs (or document why unconditional emission is required).
Suggested change
| if (child) { | |
| child->p_ptr->m_pParent = nullptr; | |
| m_objects.removeOne(child); | |
| if (child && m_objects.removeOne(child)) { | |
| child->p_ptr->m_pParent = nullptr; |
18202781743
reviewed
Apr 8, 2026
| m_page->setParent(nullptr); | ||
| m_page->deleteLater(); | ||
| m_page = nullptr; | ||
| if (m_pParent) { |
Contributor
There was a problem hiding this comment.
这里把自己从parent里删除，在下面有访问parent区删除自己，这些删除不会有相互左右么，是不是有的已经不起作用了，
if (m_parent) {
m_parent->p_ptr->removeChild(q_ptr);
}
| // StackView page transitions and tries to mark already-freed objects. | ||
| if (m_page->parent() == q_ptr) | ||
| m_page->setParent(nullptr); | ||
| m_page->deleteLater(); |
18202781743
force-pushed
the
dcc-coredump
branch
from
715fa27 to
c7f6c6b
Compare
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
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.
Change the QML creation method to asynchronous creation.
Summary by Sourcery
Switch plugin QML loading to use asynchronous incubation and adjust plugin lifecycle management accordingly.
Bug Fixes:
Enhancements: