Skip to content

Conversation

@wangrong1069
Copy link
Contributor

@wangrong1069 wangrong1069 commented Sep 4, 2025

Summary by Sourcery

Move timer thread initialization into the constructor body to ensure all members are fully initialized before starting the thread

Bug Fixes:

  • Delay timer_worker thread start until after full initialization to prevent race conditions

Chores:

  • Update Debian changelog

…tus_

backtrace:
#0  0x00007ff62581cc1b in __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:50
linuxdeepin#1  0x00007ff625807555 in __GI_abort () at abort.c:79
linuxdeepin#2  0x00007ff62580742f in __assert_fail_base
    (fmt=0x7ff621ac44f7 <error: Cannot access memory at address 0x7ff621ac44f7>, assertion=0x4a9d40 "false && \"Invalid index status\"", file=0x4a9cf0 "/build/deepin-anything-7.0.25/src/daemon/src/core/file_index_manager.cpp", line=627, function=<optimized out>) at assert.c:92
linuxdeepin#3  0x00007ff6258154d2 in __GI___assert_fail
    (assertion=0x4a9d40 "false && \"Invalid index status\"", file=0x4a9cf0 "/build/deepin-anything-7.0.25/src/daemon/src/core/file_index_manager.cpp", line=627, function=0x4aa480 "void anything::file_index_manager::save_index_status(anything::index_status)") at assert.c:101
linuxdeepin#4  0x000000000047506d in anything::file_index_manager::save_index_status(anything::index_status) ()
linuxdeepin#5  0x0000000000475106 in anything::file_index_manager::commit(anything::index_status) ()
linuxdeepin#6  0x000000000045b6f9 in base_event_handler::timer_worker(long) ()
linuxdeepin#7  0x00007ff625bf3b2f in  () at /lib/x86_64-linux-gnu/libstdc++.so.6
linuxdeepin#8  0x00007ff6264fcfa3 in start_thread (arg=<optimized out>) at pthread_create.c:486
linuxdeepin#9  0x00007ff6258de9af in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:95

Moved the initialization of the timer thread to occur after all other member variables are set up in the constructor, ensuring that the timer starts only after the event handler is fully initialized.

Log: Fix deepin-anything-daemon core dump for uninitialized index_status_
Bug: https://pms.uniontech.com/bug-view-332619.html
As title.

Log: Update version to 7.0.27
@github-actions
Copy link

github-actions bot commented Sep 4, 2025

TAG Bot

TAG: 7.0.27
EXISTED: no
DISTRIBUTION: unstable

@deepin-ci-robot
Copy link

deepin pr auto review

根据提供的代码差异,我可以看到这是一个修复 deepin-anything-daemon 核心转储(core dump)问题的补丁。主要修改涉及到 base_event_handler 类的构造函数中计时器线程的初始化方式。让我对这段代码进行详细分析:

语法逻辑

  1. 修改前,计时器线程 timer_ 在成员初始化列表中启动,这可能导致在对象完全初始化之前就开始运行 timer_worker
  2. 修改后,计时器线程的启动被移到了构造函数体内部,确保所有成员都已正确初始化后再启动线程。

代码质量

  1. 这是一个很好的改进,遵循了"在对象完全构造后再启动线程"的最佳实践。
  2. 添加的注释清晰地解释了修改的原因,提高了代码可读性。

代码性能

  1. 这个修改本身不会显著影响性能,但可以避免潜在的未定义行为。
  2. 将线程启动移到构造函数体后,确保了所有必要的资源都已准备就绪。

代码安全

  1. 原始代码存在潜在的竞态条件,因为 timer_worker 可能会在对象完全构造前访问未初始化的成员(如 index_status_)。
  2. 修改后的代码消除了这个风险,因为 index_status_ 等成员在启动线程前已经被正确初始化。

改进建议

  1. 考虑使用 std::jthread(C++20)代替 std::thread,它可以自动处理线程的join,避免在析构时忘记join的风险。
  2. 可以考虑添加一个标志位,确保 timer_worker 在构造完成前不会开始执行任何操作。
  3. 建议在构造函数中添加更多的错误处理,特别是对于线程启动可能失败的情况。

总体而言,这是一个合理的修复,解决了对象初始化顺序问题,避免了潜在的core dump。修改符合C++最佳实践,提高了代码的健壮性。

@sourcery-ai
Copy link

sourcery-ai bot commented Sep 4, 2025

Reviewer's guide (collapsed on small PRs)

Reviewer's Guide

This PR refactors the initialization of the timer thread in base_event_handler to ensure it’s launched only after the object is fully initialized, improving startup correctness.

Sequence diagram for base_event_handler initialization and timer thread launch

sequenceDiagram
    participant base_event_handler
    participant timer_thread
    base_event_handler->base_event_handler: Constructor called
    base_event_handler->base_event_handler: Initialize member variables
    base_event_handler->base_event_handler: index_dirty_ = index_manager_.refresh_indexes(...)
    base_event_handler->timer_thread: Launch timer thread after initialization
    timer_thread->base_event_handler: Execute timer_worker(1000)
Loading

Class diagram for updated base_event_handler initialization

classDiagram
    class base_event_handler {
        - batch_size_: int
        - pool_: int
        - stop_timer_: bool
        - timer_: std::thread
        - delay_mode_: bool
        - index_dirty_: bool
        - volatile_index_dirty_: bool
        - index_status_: anything::index_status
        - event_process_thread_count_: int
        + base_event_handler(std::shared_ptr<event_handler_config> config)
        + ~base_event_handler()
        + timer_worker(int interval)
    }
    base_event_handler --> event_handler_config
Loading

File-Level Changes

Change Details Files
Defer timer thread startup until after full object initialization
  • Removed timer_ initialization from the member initializer list
  • Added timer_ assignment after index refresh setup
  • Inserted comment clarifying delayed timer thread launch
src/daemon/src/core/base_event_handler.cpp

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

@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 there - I've reviewed your changes and they look great!


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

[APPROVALNOTIFIER] This PR is NOT APPROVED

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

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

@wangrong1069
Copy link
Contributor Author

/merge

@deepin-bot deepin-bot bot merged commit c40331d into linuxdeepin:develop/snipe Sep 4, 2025
18 checks passed
@wangrong1069 wangrong1069 deleted the pr0904 branch September 4, 2025 09:08
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