Bugfix: SignalTrace mode has memory and deadlock issues#3019
Merged
chenBright merged 2 commits intoapache:masterfrom Jul 18, 2025
Merged
Bugfix: SignalTrace mode has memory and deadlock issues#3019chenBright merged 2 commits intoapache:masterfrom
chenBright merged 2 commits intoapache:masterfrom
Conversation
There was a problem hiding this comment.
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
Resultstruct (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_fdsare never initialized viapipe(), so callingmake_non_blockingon -1 will fail. You need to callpipe(pipe_fds)before setting them non-blocking.
}
.github/workflows/ci-linux.yml:179
- You removed the
--cc/--cxx=clang-12flags 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
9e8e47a to
93ec09a
Compare
Contributor
|
LGTM |
Contributor
Author
|
7bd74d7 to
fa16068
Compare
fa16068 to
6ee17de
Compare
Contributor
Author
|
@wwbmmm ping! |
Contributor
|
LGTM |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What problem does this PR solve?
Issue Number: resolve #3015
Problem Summary:
What is changed and the side effects?
Changed:
Side effects:
Performance effects:
Breaking backward compatibility:
Check List: