Felix-Gong opened a new pull request, #3315:
URL: https://github.com/apache/brpc/pull/3315
## Problem
`AdaptiveMaxConcurrency` has two class-static `std::string` members
(`UNLIMITED` and `CONSTANT`) defined in `adaptive_max_concurrency.cpp`. Global
`AdaptiveMaxConcurrency` objects in other translation units may be constructed
before these static strings are initialized, causing undefined behavior
(reading uninitialized `std::string`).
This issue was discovered during RISC-V porting and testing of BRPC.
Different toolchains and linkers (GCC, Clang, cross-compilation toolchains for
RISC-V, etc.) may produce different static initialization orders, making this
bug manifest on some platforms but not others.
**Reproduction:**
```cpp
// user_code.cpp
#include "brpc/adaptive_max_concurrency.h"
static brpc::AdaptiveMaxConcurrency g_amc; // global object
int main() {
printf("g_amc.value() = \"%s\"\n", g_amc.value().c_str());
// Expected: "unlimited", Got: "" (empty string)
}
```
```
$ g++ -std=c++17 -O2 -I src -o test test_static_init_crash.cpp -L output/lib
-lbrpc ...
$ ./test
g_amc.value() = "" (len=0) # Expected: "unlimited"
```
## Fix
Replace class-static `std::string` members with Meyers' Singleton pattern
(function-local statics), which C++11 §6.7 guarantees are initialized on first
use in a thread-safe manner.
```cpp
// Before (header):
static const std::string UNLIMITED;
static const std::string CONSTANT;
// After (header):
static const std::string& UNLIMITED() {
static const std::string s_unlimited = "unlimited";
return s_unlimited;
}
static const std::string& CONSTANT() {
static const std::string s_constant = "constant";
return s_constant;
}
```
## Changes
| File | Change |
|------|--------|
| `src/brpc/adaptive_max_concurrency.h` | Static members → static member
functions |
| `src/brpc/adaptive_max_concurrency.cpp` | Remove definitions, add `()` to
calls |
| `src/brpc/server.cpp` | `UNLIMITED` → `UNLIMITED()` |
| `src/brpc/server.h` | `UNLIMITED` → `UNLIMITED()` |
| `src/brpc/policy/constant_concurrency_limiter.cpp` | `CONSTANT` →
`CONSTANT()` |
| `test/brpc_adaptive_class_unittest.cpp` | Update test references |
6 files changed, 24 insertions, 19 deletions.
## Verification
After fix, the reproducer works correctly:
```
$ ./test_static_init_crash
g_amc1.value() = "unlimited" (len=9) # Correct!
OK: static init order is fine
```
Full library build passes at 100%.
This fix benefits all architectures including x86_64, ARM64, and RISC-V.
--
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]
