Skip to content

feat: make bthread stack trace signal number configurable#3124

Merged
chenBright merged 1 commit intoapache:masterfrom
ZhengweiZhu:master
Nov 5, 2025
Merged

feat: make bthread stack trace signal number configurable#3124
chenBright merged 1 commit intoapache:masterfrom
ZhengweiZhu:master

Conversation

@ZhengweiZhu
Copy link
Copy Markdown
Contributor

@ZhengweiZhu ZhengweiZhu commented Oct 23, 2025

What problem does this PR solve?

Issue Number: resolve #3118

Problem Summary:

What is changed and the side effects?

Changed:

Side effects:

  • Performance effects:
    NO
  • Breaking backward compatibility:
    NO

Check List:

@ZhengweiZhu
Copy link
Copy Markdown
Contributor Author

@chenBright Take a look~

@wwbmmm wwbmmm requested a review from Copilot October 24, 2025 02:11
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 makes the signal number used for bthread stack tracing configurable instead of hardcoded to SIGURG. This addresses compatibility issues where SIGURG may already be registered by other libraries like cgo.

Key Changes:

  • Introduces a new flag signal_number_for_trace to configure the signal number (defaults to SIGURG for backward compatibility)
  • Updates TaskTracer to use the configured signal number instead of hardcoded SIGURG
  • Adds test coverage to verify the configuration works with alternative signals

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File Description
src/bthread/task_tracer.cpp Adds the configurable flag and updates signal registration/sending to use the configured signal number
src/bthread/task_tracer.h Adds getter for signal number and stores it as instance variable; changes RegisterSignalHandler to non-static
src/bthread/task_control.cpp Updates worker interruption to use the configured signal number instead of hardcoded SIGURG
test/brpc_builtin_service_unittest.cpp Adds test initialization to verify configuration with SIGUSR2

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Comment thread src/bthread/task_tracer.cpp
Comment thread src/bthread/task_tracer.h
@@ -100,7 +103,7 @@ class TaskTracer {
Result ContextTrace(bthread_fcontext_t fcontext);
static Result TraceByLibunwind(unw_cursor_t& cursor);

Copy link

Copilot AI Oct 24, 2025

Choose a reason for hiding this comment

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

The method signature changed from static to non-static, which is a significant API change affecting how this method is called. This change should be documented with a comment explaining why it needs to be non-static (i.e., to access the instance variable _signal_num).

Suggested change
// This method must be non-static in order to access the instance variable _signal_num.

Copilot uses AI. Check for mistakes.
Comment thread src/bthread/task_tracer.h
@ZhengweiZhu
Copy link
Copy Markdown
Contributor Author

ping~

@wwbmmm
Copy link
Copy Markdown
Contributor

wwbmmm commented Oct 30, 2025

LGTM

Copy link
Copy Markdown
Contributor

@chenBright chenBright left a comment

Choose a reason for hiding this comment

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

LGTM

@chenBright chenBright merged commit 9934c39 into apache:master Nov 5, 2025
16 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.

Initialize TaskTracer failed due to SIGURG signal handler already registered

4 participants