Skip to content

fix: prevent crash from RequestConfigFinished signal connection#145

Merged
lzwind merged 1 commit intolinuxdeepin:masterfrom
caixr23:master
Apr 20, 2026
Merged

fix: prevent crash from RequestConfigFinished signal connection#145
lzwind merged 1 commit intolinuxdeepin:masterfrom
caixr23:master

Conversation

@caixr23
Copy link
Copy Markdown
Contributor

@caixr23 caixr23 commented Apr 20, 2026

Changed the signal connection from lambda in Component.onCompleted to proper Connections component. The previous approach using a lambda directly connected to the signal could cause crashes when the component was destroyed, as the lambda might outlive the QML context or create dangling references. Using the Connections component ensures proper lifecycle management and automatic disconnection when the component is destroyed.

Influence:

  1. Test opening and closing detail configuration pages multiple times
  2. Verify no crash occurs when rapidly switching between different configuration items
  3. Test memory stability during extended usage of the configuration tool
  4. Verify signal disconnection works correctly when components are destroyed
  5. Test that globalConfigOptions are properly refreshed when RequestConfigFinished is emitted

fix: 修复 RequestConfigFinished 信号导致崩溃的问题

将信号连接方式从 Component.onCompleted 中的 lambda 改为使用 Connections 组件。之前直接在信号上连接 lambda 的方式可能在组件销毁时导致崩溃,因为
lambda 可能超出 QML 上下文的生命周期或产生悬空引用。使用 Connections 组
件可以确保正确的生命周期管理,并在组件销毁时自动断开连接。

Influence:

  1. 测试多次打开和关闭详细配置页面
  2. 验证在不同配置项之间快速切换时不会发生崩溃
  3. 测试配置工具长时间使用时的内存稳定性
  4. 验证组件销毁时信号断开连接是否正常工作
  5. 测试 RequestConfigFinished 发出时 globalConfigOptions 是否正确刷新

Summary by Sourcery

Prevent configuration detail view crashes by changing how the RequestConfigFinished signal is handled in the QML detail config item.

Bug Fixes:

  • Use a QML Connections handler for RequestConfigFinished to avoid crashes caused by a lambda tied to the component lifecycle in Component.onCompleted.

Enhancements:

  • Ensure configuration options and key name are safely reset and refreshed when RequestConfigFinished is emitted while preserving the initial configuration load on component completion.

Changed the signal connection from lambda in Component.onCompleted
to proper Connections component. The previous approach using a lambda
directly connected to the signal could cause crashes when the component
was destroyed, as the lambda might outlive the QML context or create
dangling references. Using the Connections component ensures proper
lifecycle management and automatic disconnection when the component
is destroyed.

Influence:
1. Test opening and closing detail configuration pages multiple times
2. Verify no crash occurs when rapidly switching between different
configuration items
3. Test memory stability during extended usage of the configuration tool
4. Verify signal disconnection works correctly when components are
destroyed
5. Test that globalConfigOptions are properly refreshed when
RequestConfigFinished is emitted

fix: 修复 RequestConfigFinished 信号导致崩溃的问题

将信号连接方式从 Component.onCompleted 中的 lambda 改为使用 Connections
组件。之前直接在信号上连接 lambda 的方式可能在组件销毁时导致崩溃,因为
lambda 可能超出 QML 上下文的生命周期或产生悬空引用。使用 Connections 组
件可以确保正确的生命周期管理,并在组件销毁时自动断开连接。

Influence:
1. 测试多次打开和关闭详细配置页面
2. 验证在不同配置项之间快速切换时不会发生崩溃
3. 测试配置工具长时间使用时的内存稳定性
4. 验证组件销毁时信号断开连接是否正常工作
5. 测试 RequestConfigFinished 发出时 globalConfigOptions 是否正确刷新
@sourcery-ai
Copy link
Copy Markdown

sourcery-ai Bot commented Apr 20, 2026

Reviewer's guide (collapsed on small PRs)

Reviewer's Guide

Refactors the handling of the RequestConfigFinished signal in DetailConfigItem.qml from an inline lambda connection in Component.onCompleted to a dedicated QML Connections object to ensure proper lifecycle handling and prevent crashes when the component is destroyed, while preserving the existing behavior of refreshing configuration options.

Sequence diagram for handling RequestConfigFinished with QML Connections

sequenceDiagram
    actor User
    participant DetailConfigItem
    participant Fcitx5ConfigProxy

    activate DetailConfigItem
    User->>DetailConfigItem: Open_detail_config_page
    DetailConfigItem->>Fcitx5ConfigProxy: globalConfigOptions(root.name)
    Fcitx5ConfigProxy-->>DetailConfigItem: initial_configOptions
    Note over DetailConfigItem: Component.onCompleted sets
    Note over DetailConfigItem: configOptions and loading false

    User->>Fcitx5ConfigProxy: Change_configuration_and_request_refresh
    Fcitx5ConfigProxy-->>Fcitx5ConfigProxy: Process_request
    Fcitx5ConfigProxy-->>DetailConfigItem: RequestConfigFinished
    DetailConfigItem-->>DetailConfigItem: Connections.onRequestConfigFinished
    DetailConfigItem->>Fcitx5ConfigProxy: globalConfigOptions(root.name)
    Fcitx5ConfigProxy-->>DetailConfigItem: updated_configOptions
    DetailConfigItem-->>DetailConfigItem: configOptions reset_and_refreshed
    DetailConfigItem-->>DetailConfigItem: keyName set_to_empty_string

    User->>DetailConfigItem: Close_detail_config_page
    deactivate DetailConfigItem
    Note over DetailConfigItem,Fcitx5ConfigProxy: Connections auto disconnects
    Note over DetailConfigItem,Fcitx5ConfigProxy: No dangling lambda or crash
Loading

Flow diagram for DetailConfigItem signal lifecycle with Connections

flowchart TD
    A[DetailConfigItem created] --> B[Component.onCompleted]
    B --> C[Fetch initial globalConfigOptions from Fcitx5ConfigProxy]
    C --> D[Set configOptions and loading false]
    A --> E[Connections target dccData.fcitx5ConfigProxy]
    E --> F[Connect onRequestConfigFinished handler]

    Fcitx[RequestConfigFinished emitted by Fcitx5ConfigProxy] --> G[Connections.onRequestConfigFinished]
    G --> H[Set configOptions to empty]
    H --> I[Fetch globalConfigOptions root.name from Fcitx5ConfigProxy]
    I --> J[Update configOptions]
    J --> K[Set keyName to empty string]

    L[DetailConfigItem destroyed] --> M[Connections automatically disconnected]
    M --> N[No access to destroyed QML context]
    N --> O[Crash from dangling lambda prevented]
Loading

File-Level Changes

Change Details Files
Use a QML Connections object instead of an inline lambda to handle the RequestConfigFinished signal, ensuring automatic disconnection on component destruction and avoiding crashes from dangling references.
  • Add a Connections block targeting dccData.fcitx5ConfigProxy with an onRequestConfigFinished handler that clears and repopulates configOptions and resets keyName
  • Remove the lambda-based onRequestConfigFinished.connect call from Component.onCompleted
  • Keep initial loading logic in Component.onCompleted to populate configOptions on component creation and set loading to false
src/dcc-fcitx5configtool/qml/DetailConfigItem.qml

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

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.

Hey - I've left some high level feedback:

  • The logic that clears and reloads configOptions and keyName is now duplicated between onRequestConfigFinished and Component.onCompleted; consider extracting this into a small helper function or method that both places call to keep behavior in sync.
  • If dccData.fcitx5ConfigProxy can ever be null or change during the component’s lifecycle, it may be safer to guard the Connections target (e.g. using a ternary or binding) to avoid binding to an invalid object.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- The logic that clears and reloads `configOptions` and `keyName` is now duplicated between `onRequestConfigFinished` and `Component.onCompleted`; consider extracting this into a small helper function or method that both places call to keep behavior in sync.
- If `dccData.fcitx5ConfigProxy` can ever be null or change during the component’s lifecycle, it may be safer to guard the `Connections` target (e.g. using a ternary or binding) to avoid binding to an invalid object.

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

@deepin-ci-robot
Copy link
Copy Markdown

deepin pr auto review

这段代码的修改主要涉及 QML 中信号连接方式的变更,将内联的 JavaScript 函数连接改为了更规范的 Connections 对象方式。以下是对这段 diff 的详细审查和改进建议:

1. 语法逻辑审查

当前逻辑:

  • 新增了一个 Connections 对象来监听 dccData.fcitx5ConfigProxyonRequestConfigFinished 信号。
  • 当信号触发时,重置 configOptionskeyName
  • Component.onCompleted 中移除了旧的 .connect() 写法,但保留了初始化数据获取的逻辑。

潜在逻辑问题:

  • 初始化时机与信号处理的冲突Component.onCompleted 中直接调用了 globalConfigOptions,而 Connections 中也调用了相同的函数。如果 onRequestConfigFinished 信号在 Component.onCompleted 之后触发,会导致 configOptions 被重新赋值,这可能不是预期行为(取决于业务逻辑)。
  • 重复赋值:在 Connections 的处理函数中,先执行 configOptions = [] 再执行 configOptions = ...,中间的空数组赋值是不必要的,且可能导致 UI 短暂闪烁。

2. 代码质量审查

优点:

  • 使用 Connections 对象比 .connect() 更符合 QML 的惯用写法,可读性更好。
  • 信号连接的生命周期管理更清晰(随 Connections 对象销毁而自动断开)。

缺点:

  • Connections 的目标对象 dccData.fcitx5ConfigProxy 可能是动态创建的,如果该对象被销毁重建,Connections 不会自动重新连接(除非 Connections 也被重建)。
  • 缺少对 target 是否为 null 的检查,如果 dccData.fcitx5ConfigProxy 尚未初始化,可能会报错。

3. 代码性能审查

  • 不必要的空数组赋值configOptions = [] 会触发属性绑定的重新计算,如果 configOptions 是一个大型列表,这会导致不必要的性能开销。
  • 重复的数据获取:如果 onRequestConfigFinished 信号频繁触发,每次都会重新获取配置,可能导致性能问题(取决于 globalConfigOptions 的实现)。

4. 代码安全审查

  • 目标对象有效性检查:未检查 dccData.fcitx5ConfigProxy 是否存在,如果该对象为 null,会导致运行时错误。
  • 信号连接的内存泄漏风险:虽然 Connections 会自动管理生命周期,但如果 target 对象频繁销毁重建,仍需确保 Connections 对象也被正确管理。

改进建议

以下是改进后的代码示例,附带详细注释:

DccObject {
    // ... 其他代码 ...

    Connections {
        target: dccData.fcitx5ConfigProxy
        // 显式声明目标对象的有效性(可选,取决于业务需求)
        enabled: dccData.fcitx5ConfigProxy !== null

        function onRequestConfigFinished() {
            // 直接赋值,避免中间的空数组操作
            const newOptions = dccData.fcitx5ConfigProxy.globalConfigOptions(root.name)
            // 只有当新数据与旧数据不同时才更新(避免不必要的 UI 刷新)
            if (JSON.stringify(configOptions) !== JSON.stringify(newOptions)) {
                configOptions = newOptions
                keyName = ""
            }
        }
    }

    Component.onCompleted: {
        if (dccData.fcitx5ConfigProxy) {
            // 初始化时直接获取配置,不依赖信号
            configOptions = dccData.fcitx5ConfigProxy.globalConfigOptions(root.name)
            loading = false
        }
    }
}

改进点说明:

  1. 目标对象检查:添加 enabled: dccData.fcitx5ConfigProxy !== null 确保 Connections 只在目标对象有效时工作。
  2. 避免不必要的赋值:直接将结果赋值给 configOptions,而不是先清空再赋值。
  3. 数据变化检测:通过 JSON.stringify 比较新旧数据,避免不必要的更新(如果数据量很大,可以优化比较逻辑)。
  4. 初始化安全:在 Component.onCompleted 中检查目标对象是否存在。

进一步优化建议:

  • 如果 globalConfigOptions 是一个耗时操作,建议将其改为异步调用或通过信号返回结果。
  • 如果 configOptions 是一个大型列表,考虑使用 ListViewcacheBufferdelegate 优化渲染性能。
  • 如果 onRequestConfigFinished 信号可能频繁触发,可以添加防抖(debounce)逻辑。

希望这些建议对您有所帮助!

@deepin-ci-robot
Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: caixr23, lzwind

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

@lzwind lzwind merged commit 5261658 into linuxdeepin:master Apr 20, 2026
11 of 12 checks passed
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.

3 participants