walterzhaoJR opened a new pull request, #3414: URL: https://github.com/apache/brpc/pull/3414
Replace the recursive pthread mutex protecting bvar's VarMap with a bthread-aware recursive mutex so yielding metric callbacks do not block worker pthreads. Add regression coverage for worker starvation, recursive ownership across bthread migration, pthread use, and high contention. Related issue: https://github.com/apache/brpc/issues/2888 ### What problem does this PR solve? Issue Number: resolves #2888 Problem Summary: `bvar::Variable` protects its global VarMap with a recursive pthread mutex. If a metric callback, such as a `PassiveStatus` getter, yields while holding this lock, other bthreads may block their worker pthreads while waiting for it. Once all workers are blocked, the lock holder cannot resume, causing worker starvation or deadlock. A pthread recursive mutex also identifies its owner by OS pthread, which does not correctly represent a bthread that may migrate between workers. ### What is changed and the side effects? Changed: - Add `bthread_recursive_mutex_t` and its C++ wrapper, `bthread::RecursiveMutex`. - Track ownership by bthread ID for bthreads while retaining support for native pthread callers. - Use `bthread_mutex_t` for contention so waiting bthreads are suspended instead of blocking worker pthreads. - Replace the recursive pthread mutex protecting bvar's VarMap with `bthread::RecursiveMutex`. - Add regression tests covering: - bvar worker starvation; - recursive ownership across bthread yields and worker migration; - native pthread and C++ wrapper usage; - high-contention recursive locking. - Keep the standalone `test_bvar` target independent of the full bthread runtime. Side effects: - Performance effects: - Recursive lock operations add small owner and recursion-depth checks. - Under contention, logical bthreads are suspended instead of blocking worker pthreads. - The stress test completes 640,000 critical-section operations while exercising frequent cross-worker migration. - Breaking backward compatibility: - No. This change adds new APIs and only replaces an internal lock used by bvar's VarMap. ### Test results - `bthread_mutex_unittest`: 3 recursive mutex tests passed. - `brpc_bvar_mutex_unittest`: 3 bvar regression tests passed. - `test_bvar`: 64/64 tests passed. - All related targets compiled successfully with GCC 10. --- ### Check List: - Please make sure your changes are compilable. - When providing us with a new feature, it is best to add related tests. - Please follow [Contributor Covenant Code of Conduct](https://github.com/apache/brpc/blob/master/CODE_OF_CONDUCT.md). -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
