Skip to content

refactor: split PluginManager into DccPluginLoader and DccPluginManager#3205

Open
caixr23 wants to merge 1 commit intolinuxdeepin:masterfrom
caixr23:master
Open

refactor: split PluginManager into DccPluginLoader and DccPluginManager#3205
caixr23 wants to merge 1 commit intolinuxdeepin:masterfrom
caixr23:master

Conversation

@caixr23
Copy link
Copy Markdown
Contributor

@caixr23 caixr23 commented Apr 29, 2026

Extract plugin loading state machine from PluginData into DccPluginLoader class with signal-driven auto-advancing states. Simplify PluginManager (renamed to DccPluginManager) to only handle scheduling, thread pool and lifecycle management. Fix display module unnecessary delayed initialization and adjust root/engine deletion order in cleanup.

将PluginData中的插件加载逻辑提取为DccPluginLoader类,使用信号驱动状态机自动推进。 简化PluginManager(重命名为DccPluginManager)为调度协调角色,仅管理线程池和生命周期。 修复DisplayModule不必要的延迟初始化,调整清理时root/engine的删除顺序。

Log: 重构插件加载架构,分离加载逻辑与调度管理
Influence: 1. 控制中心各模块正常加载和显示;2. 插件隐藏/取消隐藏功能正常;3. 异步数据加载正常;4. 模块切换和页面导航正常

Copy link
Copy Markdown

@sourcery-ai sourcery-ai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry @caixr23, you have reached your weekly rate limit of 500000 diff characters.

Please try again later or upgrade to continue using Sourcery

@deepin-ci-robot
Copy link
Copy Markdown

[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.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@caixr23 caixr23 force-pushed the master branch 2 times, most recently from 4f4570c to cd0cf68 Compare April 29, 2026 09:06
Extract plugin loading state machine from PluginData into DccPluginLoader class
with signal-driven auto-advancing states. Simplify PluginManager (renamed to
DccPluginManager) to only handle scheduling, thread pool and lifecycle management.
Fix display module unnecessary delayed initialization and adjust root/engine
deletion order in cleanup.

将PluginData中的插件加载逻辑提取为DccPluginLoader类,使用信号驱动状态机自动推进。
简化PluginManager(重命名为DccPluginManager)为调度协调角色,仅管理线程池和生命周期。
修复DisplayModule不必要的延迟初始化,调整清理时root/engine的删除顺序。

Log: 重构插件加载架构,分离加载逻辑与调度管理
Influence: 1. 控制中心各模块正常加载和显示;2. 插件隐藏/取消隐藏功能正常;3. 异步数据加载正常;4. 模块切换和页面导航正常
@deepin-ci-robot
Copy link
Copy Markdown

deepin pr auto review

代码审查报告

整体概述

这次代码变更主要是将插件管理功能从PluginManager重构为更模块化的DccPluginManagerDccPluginLoader,同时调整了DccManager类以使用新的插件管理器。此外,还修改了DisplayModulePrivate的初始化逻辑。

代码质量与架构改进

1. 模块化设计改进

优点

  • 将插件管理功能拆分为DccPluginManagerDccPluginLoader两个类,职责更加清晰
    • DccPluginManager负责整体插件加载流程管理
    • DccPluginLoader负责单个插件的加载过程
  • 使用状态机模式管理插件加载状态,通过StatusFlagsTypeFlags枚举清晰定义状态
  • 将插件数据从结构体(PluginData)转换为类(DccPluginLoader),更好地封装了插件加载逻辑

建议

  • 考虑为DccPluginLoader添加更详细的文档说明其状态转换流程
  • 可以考虑将状态转换逻辑进一步抽象为状态模式,使状态转换更加明确

2. 代码复用与一致性

优点

  • 统一了插件加载流程,减少了重复代码
  • 使用QThreadPool进行异步加载,提高了加载效率

问题

  • DccPluginLoaderDccPluginManager之间存在紧密耦合,DccPluginLoader需要持有DccPluginManager的指针
  • dccpluginloader.cpp中,loadModule()loadMain()方法有相似的逻辑,可以考虑提取公共部分

3. 错误处理与日志

优点

  • 使用Q_LOGGING_CATEGORY进行分类日志记录,便于调试
  • 在关键操作处添加了状态转换日志,便于跟踪问题

问题

  • 错误处理不够一致,有些地方使用setLog记录错误,有些地方直接使用qCWarning
  • loadData()方法中,加载失败后没有清理已加载的QPluginLoader对象,可能导致资源泄漏

性能考虑

优点

  • 使用线程池异步加载数据,不阻塞主线程
  • 优先加载"system"和"device"插件,优化了用户体验

问题

  • loadPlugin()方法中,每次状态变化都会触发信号,可能导致频繁的信号槽调用
  • updateType()方法在尝试不同版本时会递归调用自身,可能导致性能问题

安全问题

问题

  1. dccpluginloader.cpploadData()方法中,加载动态库时没有进行安全检查:

    QPluginLoader loader(soPath);
    if (!loader.load()) {
        setLog("prepare factory load failed:" + loader.errorString());
        transitionStatus(DataErr);
        return;
    }

    建议添加对.so文件的来源验证或完整性检查。

  2. dccpluginloader.cpploadModule()loadMain()方法中,直接从文件路径加载QML组件,存在路径遍历风险:

    const QString qmlPath = m_path + "/" + m_name + ".qml";
    component.loadUrl(qmlPath);

    建议对路径进行规范化处理,确保不会加载预期之外的文件。

  3. dccpluginloader.cppupdateType()方法中,递归调用自身可能导致栈溢出:

    default: {
        // Try to determine version
        m_type = T_V1_1;
        if (updateType()) {
            return true;
        }
        m_type = T_V1_0;
        if (updateType()) {
            return true;
        }
        m_type = T_Unknown;
        return false;
    } break;

其他问题

  1. dccmanager.cppclearData()方法中,调整了删除顺序,但注释中的"TODO: delete m_engine会有概率崩溃"问题仍未解决,需要进一步调查。

  2. dccpluginloader.cpploadMain()方法中,创建QQmlContext时没有设置父对象,可能导致内存泄漏:

    QQmlContext *context = new QQmlContext(component.engine());
    context->setContextProperties({ { "dccData", QVariant::fromValue(m_data) }, { "dccModule", QVariant::fromValue(m_module) } });
    QObject *object = component.create(context);
    if (!object) {
        delete context;
        setLog(" component create main object is null:" + component.errorString());
        transitionStatus(MainObjErr);
        return;
    }
    context->setParent(object); // Context will be deleted when object is deleted

    虽然在成功时设置了父对象,但在错误情况下才删除context,逻辑不够一致。

  3. displaymodule.cpp中,初始化逻辑从异步改为同步:

    // 修改前
    QMetaObject::invokeMethod(
            q_ptr,
            [this]() {
                init();
            },
            Qt::QueuedConnection);
    
    // 修改后
    m_model = new DisplayModel(q_ptr);
    m_worker = new DisplayWorker(m_model, q_ptr);
    init();

    这个改动可能会影响启动性能,需要评估其影响。

建议改进

  1. 错误处理改进

    • 统一错误处理机制,考虑使用错误码或异常处理
    • 在加载失败时确保清理所有已分配的资源
  2. 安全性增强

    • 添加插件来源验证和完整性检查
    • 对文件路径进行规范化处理,防止路径遍历攻击
    • 考虑使用沙箱机制隔离插件加载
  3. 性能优化

    • 减少不必要的信号槽调用
    • 优化递归调用,避免栈溢出风险
    • 考虑延迟加载非关键插件
  4. 代码结构改进

    • 进一步解耦DccPluginLoaderDccPluginManager
    • 提取loadModule()loadMain()中的公共代码
    • 为状态转换添加更详细的文档
  5. 测试建议

    • 添加单元测试覆盖各种加载状态和错误情况
    • 进行性能测试,评估插件加载时间
    • 进行安全测试,验证插件加载的安全性

总结

这次重构在代码结构和模块化方面有显著改进,将插件管理功能拆分为更清晰的组件。但在错误处理、安全性和性能方面仍有一些改进空间。建议在后续版本中逐步解决这些问题,特别是安全相关的改进应该优先考虑。

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants