Use SetNameSimple instead of SetName to avoid deadlock#3127
Use SetNameSimple instead of SetName to avoid deadlock#3127chenBright merged 1 commit intoapache:masterfrom
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR addresses a deadlock issue (#3104) by replacing PlatformThread::SetName calls with a new SetNameSimple variant across various thread initialization functions. The change introduces SetNameSimple as a lighter-weight alternative that sets only the OS-level thread name without the additional overhead of ThreadIdNameManager operations that could cause deadlocks.
- Introduces
SetNameSimplemethod that bypassesThreadIdNameManagerand tracked objects - Refactors
SetNameto callSetNameSimpleinternally, maintaining existing functionality - Updates all thread entry points to use
SetNameSimpleinstead ofSetName
Reviewed Changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/butil/threading/platform_thread.h | Declares new SetNameSimple static method |
| src/butil/threading/platform_thread_linux.cc | Implements SetNameSimple and refactors SetName to use it |
| src/butil/threading/platform_thread_mac.mm | Implements SetNameSimple and refactors SetName to use it |
| src/butil/threading/platform_thread_freebsd.cc | Implements SetNameSimple and refactors SetName to use it |
| src/bvar/variable.cpp | Updates bvar_dumper thread to use SetNameSimple |
| src/bvar/detail/sampler.cpp | Updates bvar_sampler thread to use SetNameSimple |
| src/bvar/collector.cpp | Updates collector grabber and dumper threads to use SetNameSimple |
| src/bthread/timer_thread.cpp | Updates brpc_timer thread to use SetNameSimple |
| src/bthread/task_control.cpp | Updates worker threads to use SetNameSimple |
| src/bthread/execution_queue.cpp | Updates ExecutionQueue thread to use SetNameSimple |
| src/brpc/details/usercode_backup_pool.cpp | Updates user code runner thread to use SetNameSimple |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
why ThreadIdNameManager case deadlock |
Still not sure why. It's strange that |
|
LGTM |
1 similar comment
|
LGTM |
In the sampling thread, after SetName enters the lock, the main thread performs a fork. When the child process acquires memory, it is locked, causing all threads in the child process to get stuck. |
What problem does this PR solve?
Issue Number: resolve #3104
Problem Summary:
What is changed and the side effects?
Changed:
Side effects:
Performance effects:
Breaking backward compatibility:
Check List: