Skip to content

Bugfix: SignalTrace mode has memory and deadlock issues#3019

Merged
chenBright merged 2 commits intoapache:masterfrom
chenBright:fix_task_tracer
Jul 18, 2025
Merged

Bugfix: SignalTrace mode has memory and deadlock issues#3019
chenBright merged 2 commits intoapache:masterfrom
chenBright:fix_task_tracer

Conversation

@chenBright
Copy link
Copy Markdown
Contributor

@chenBright chenBright commented Jul 6, 2025

What problem does this PR solve?

Issue Number: resolve #3015

Problem Summary:

  1. After the signal handler times out, the context may become invalid.
  2. SignalSync destructor in signal handler may cause deadlock.

What is changed and the side effects?

Changed:

  1. Use async-signal-safe absl instead of libunwind to trace the stack in the signal handler.
  2. Hold additional ref of SignalSync to avoid SignalSync destructor in signal handler.

Side effects:

  • Performance effects:

  • Breaking backward compatibility:


Check List:

@chenBright chenBright requested a review from Copilot July 6, 2025 15:31
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR refactors the bthread stack tracing implementation to use Abseil’s async-signal-safe unwinder, removes the old enable_fast_unwind mode, simplifies Result, and updates build scripts and docs to pull in Abseil dependencies.

  • Replace libunwind‐based fast/slow unwind toggling with a single Abseil unwinder (DefaultStackUnwinder)
  • Simplify Result struct (single error buffer, remove semaphore/traditional pipe sync)
  • Update CMake, Bazel, shell scripts, and CI workflows to add Abseil dependencies and flags

Reviewed Changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
test/bthread_unittest.cpp Remove fast/unwind flag tests, adjust loop break logic
src/bthread/task_tracer.h Simplify Result, remove semaphore, add _inuse_signal_syncs
src/bthread/task_tracer.cpp Remove old signal sync, implement Abseil unwinder, update APIs
docs/cn/bthread_tracer.md Update installation steps to include Abseil
config_brpc.sh Add Abseil header/lib discovery
MODULE.bazel Normalize quotes, add Abseil deps
CMakeLists.txt find_package(absl) and link Abseil targets
BUILD.bazel Add Abseil debug libraries
.github/workflows/ci-linux.yml Update CI flags for tracer
.github/actions/... Install Abseil in custom actions
Comments suppressed due to low confidence (2)

src/bthread/task_tracer.cpp:62

  • pipe_fds are never initialized via pipe(), so calling make_non_blocking on -1 will fail. You need to call pipe(pipe_fds) before setting them non-blocking.
    }

.github/workflows/ci-linux.yml:179

  • You removed the --cc/--cxx=clang-12 flags from this step. If your build assumes clang-12, CI may now compile with the system default compiler, potentially causing failures. Re-add or confirm the intended compiler flags.
        options: --with-bthread-tracer

Comment thread test/bthread_unittest.cpp
Comment thread src/bthread/task_tracer.cpp Outdated
Comment thread src/bthread/task_tracer.cpp Outdated
@wwbmmm
Copy link
Copy Markdown
Contributor

wwbmmm commented Jul 7, 2025

LGTM

@chenBright
Copy link
Copy Markdown
Contributor Author

chenBright commented Jul 7, 2025

  1. FIx memory leak of SignalSync.
  2. Fix wrong status of global priority bthread.

@chenBright
Copy link
Copy Markdown
Contributor Author

@wwbmmm ping!

@wwbmmm
Copy link
Copy Markdown
Contributor

wwbmmm commented Jul 14, 2025

LGTM

@chenBright chenBright merged commit bea48d7 into apache:master Jul 18, 2025
15 checks passed
@chenBright chenBright deleted the fix_task_tracer branch July 18, 2025 15:35
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.

task tracer 信号追踪问题讨论

3 participants