refactor: unify dde-apps loading in main thread - #3456
18202781743 wants to merge 1 commit into
Conversation
Load the dde-apps applet once in DccManager during initialization instead of having each plugin create its own instance. This centralizes lifecycle management, avoids duplicate applet creation across plugins, and ensures applet initialization happens on the main thread natively without workarounds. Removed redundant applet creation logic from notification AppMgr and privacy PrivacySecurityWorker, along with the BlockingQueuedConnection workaround previously needed for thread safety. 1. Add Dde Shell dependency to CMakeLists for control center 2. Create and load dde-apps applet in DccManager::init() 3. Remove duplicated applet creation from AppMgr 4. Simplify PrivacySecurityWorker to use existing applet 5. Replace BlockingQueuedConnection with direct method call 6. Remove obsolete thread-related comments and includes Log: Unify dde-apps loading in main thread for control center initialization Influence: 1. Verify control center starts without errors 2. Test DCC main window loads completely in main thread 3. Check plugin functionality that depends on dde-apps (notification, privacy) 4. Verify no duplicate applet instances are created 5. Test applet initialization order and performance 6. Confirm thread safety with no crashes during startup refactor: 统一在主线程中加载dde-apps 在DccManager初始化阶段统一加载dde-apps组件,取代各插件自行创建实例的方 式。这集中了生命周期管理,避免跨插件的重复创建,并确保组件在主线程原生初 始化,无需额外线程处理。 移除了通知模块AppMgr和隐私模块PrivacySecurityWorker中重复的组件创建逻 辑,以及此前为线程安全而添加的BlockingQueuedConnection处理。 1. 在CMakeLists中添加Dde Shell依赖 2. 在DccManager::init()中创建并加载dde-apps组件 3. 移除AppMgr中的重复组件创建 4. 简化PrivacySecurityWorker使用现有组件 5. 将BlockingQueuedConnection替换为直接方法调用 6. 移除过时的线程相关注释和头文件 Log: 统一在主线程中加载dde-apps,优化控制中心初始化 Influence: 1. 验证控制中心启动无报错 2. 测试主窗口在主线程中完整加载 3. 检查依赖dde-apps的插件功能(通知、隐私) 4. 确认未创建重复组件实例 5. 测试组件初始化顺序和性能 6. 确认启动期间无线程安全问题崩溃
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: 18202781743 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 GuideThe control center now loads and initializes a single dde-apps applet during DccManager initialization on the main thread, while notification and privacy plugins consume the existing proxy without creating duplicate instances or using thread workarounds. Sequence diagram for centralized dde-apps initializationsequenceDiagram
participant DccManager
participant DPluginLoader
participant DContainment
participant DdeAppsApplet
participant NotificationAppMgr
participant PrivacySecurityWorker
participant DAppletBridge
DccManager->>DccManager: init()
DccManager->>DPluginLoader: rootApplet()
DPluginLoader-->>DccManager: DContainment
DccManager->>DContainment: createApplet(DAppletData)
DContainment-->>DccManager: DdeAppsApplet
DccManager->>DdeAppsApplet: load()
DccManager->>DdeAppsApplet: init()
NotificationAppMgr->>DAppletBridge: applet()
DAppletBridge-->>NotificationAppMgr: existing applet proxy
PrivacySecurityWorker->>PrivacySecurityWorker: init()
PrivacySecurityWorker->>PrivacySecurityWorker: initApp()
PrivacySecurityWorker->>DAppletBridge: applet()
DAppletBridge-->>PrivacySecurityWorker: existing applet proxy
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 found 1 issue
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="src/plugin-privacy/operation/privacysecurityworker.cpp" line_range="88" />
<code_context>
- // TODO:由于控制中心通过子线程加载插件后,会移动插件的加载线程->主线程,
- // 并删除原有线程->未指定父的对象所处的线程会被删除,所以使用qApp->主线程调用initApp
- QMetaObject::invokeMethod(qApp, [this]() {
- initApp();
- }, Qt::BlockingQueuedConnection);
-
</code_context>
<issue_to_address>
**issue (bug_risk):** `initApp()` now runs directly in the plugin data-loading thread, while `DAppletProxy` and its `appModel` belong to the main thread where `DccManager::loadAppInfos()` created and initialized the applet. The worker therefore accesses the applet proxy/model and installs signal connections from the wrong thread, reintroducing the thread-affinity problem that the removed `BlockingQueuedConnection` avoided.
**Triggers:** When the privacy plugin is created by `DccPluginManager::LoadDataTask` before `moveThread()` moves the plugin objects.
**Suggested fix:** Keep invoking `initApp()` on the application thread, or otherwise marshal all applet/model access to the applet's owning thread.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| initApp(); | ||
| }, Qt::BlockingQueuedConnection); | ||
|
|
||
| initApp(); |
There was a problem hiding this comment.
issue (bug_risk): initApp() now runs directly in the plugin data-loading thread, while DAppletProxy and its appModel belong to the main thread where DccManager::loadAppInfos() created and initialized the applet. The worker therefore accesses the applet proxy/model and installs signal connections from the wrong thread, reintroducing the thread-affinity problem that the removed BlockingQueuedConnection avoided.
Triggers: When the privacy plugin is created by DccPluginManager::LoadDataTask before moveThread() moves the plugin objects.
Suggested fix: Keep invoking initApp() on the application thread, or otherwise marshal all applet/model access to the applet's owning thread.
deepin pr auto review🤖 AI 代码审查报告📊 总体评价
🔍 详细分析1. 语法逻辑 ✅评价: 优秀 ✅ 通过 潜在问题: 建议: 建议在 appmgr.cpp 中添加注释说明 dde-apps 已由 DccManager::loadAppInfos() 在主线程统一加载,此处仅获取已加载的 applet proxy 2. 代码质量 ✅评价: 优秀 ✅ 通过 潜在问题:
建议: 1.将 qWarning() 替换为 qCWarning(dccLog()) 保持日志分类一致性 3. 代码性能 ✅评价: 优秀 ✅ 通过 潜在问题: 建议: 性能优化合理,无需额外改进 4. 代码安全 🔒评价: 优秀 ✅ 通过
安全漏洞详情: 建议: 代码安全合规,无安全风险 💡 改进建议代码示例// dccmanager.cpp - 建议修改
void DccManager::loadAppInfos()
{
// 在主线程统一加载 dde-apps,供各插件通过 DAppletBridge 获取
DCC_BENCHMARK("dde-apps", "Load-dde-apps");
auto rootApplet = qobject_cast<DS_NAMESPACE::DContainment *>(DS_NAMESPACE::DPluginLoader::instance()->rootApplet());
if (!rootApplet) {
qCWarning(dccLog()) << "Failed to get root applet for dde-apps";
return;
}
auto applet = rootApplet->createApplet(DS_NAMESPACE::DAppletData{"org.deepin.ds.dde-apps"});
if (!applet) {
qCWarning(dccLog()) << "Failed to create dde-apps applet";
return;
}
applet->load();
applet->init();
}本报告由 AI 代码审查工具自动生成 |
Load the dde-apps applet once in DccManager during initialization instead of having each plugin create its own instance. This centralizes lifecycle management, avoids duplicate applet creation across plugins, and ensures applet initialization happens on the main thread natively without workarounds.
Removed redundant applet creation logic from notification AppMgr and privacy PrivacySecurityWorker, along with the BlockingQueuedConnection workaround previously needed for thread safety.
Log: Unify dde-apps loading in main thread for control center initialization
Influence:
refactor: 统一在主线程中加载dde-apps
在DccManager初始化阶段统一加载dde-apps组件,取代各插件自行创建实例的方
式。这集中了生命周期管理,避免跨插件的重复创建,并确保组件在主线程原生初
始化,无需额外线程处理。
移除了通知模块AppMgr和隐私模块PrivacySecurityWorker中重复的组件创建逻
辑,以及此前为线程安全而添加的BlockingQueuedConnection处理。
Log: 统一在主线程中加载dde-apps,优化控制中心初始化
Influence:
Summary by Sourcery
Load the dde-apps applet centrally during control center initialization to provide consistent lifecycle management and eliminate duplicate plugin initialization.
Enhancements:
Build: