This is an automated email from the ASF dual-hosted git repository.
chenBright pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/brpc.git
The following commit(s) were added to refs/heads/master by this push:
new e9d4b190 Fix static initialization order fiasco in
AdaptiveMaxConcurrency (#3315)
e9d4b190 is described below
commit e9d4b190fb7198fad0f9a296905fe33b761fb818
Author: Felix-Gong <[email protected]>
AuthorDate: Fri May 29 15:29:04 2026 +0800
Fix static initialization order fiasco in AdaptiveMaxConcurrency (#3315)
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.
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.
Fix by replacing class-static std::string members with Meyers' Singleton
pattern (function-local statics), which C++11 guarantees are initialized
on first use in a thread-safe manner.
This fix benefits all architectures including x86_64, ARM64, and RISC-V.
Signed-off-by: Felix-Gong <[email protected]>
---
src/brpc/adaptive_max_concurrency.cpp | 13 +++++--------
src/brpc/adaptive_max_concurrency.h | 14 +++++++++++---
src/brpc/policy/constant_concurrency_limiter.cpp | 2 +-
src/brpc/server.cpp | 4 ++--
src/brpc/server.h | 2 +-
test/brpc_adaptive_class_unittest.cpp | 8 ++++----
6 files changed, 24 insertions(+), 19 deletions(-)
diff --git a/src/brpc/adaptive_max_concurrency.cpp
b/src/brpc/adaptive_max_concurrency.cpp
index d1be2730..6a5977bc 100644
--- a/src/brpc/adaptive_max_concurrency.cpp
+++ b/src/brpc/adaptive_max_concurrency.cpp
@@ -25,18 +25,15 @@
namespace brpc {
-const std::string AdaptiveMaxConcurrency::UNLIMITED = "unlimited";
-const std::string AdaptiveMaxConcurrency::CONSTANT = "constant";
-
AdaptiveMaxConcurrency::AdaptiveMaxConcurrency()
- : _value(UNLIMITED)
+ : _value(UNLIMITED())
, _max_concurrency(0) {
}
AdaptiveMaxConcurrency::AdaptiveMaxConcurrency(int max_concurrency)
: _max_concurrency(0) {
if (max_concurrency <= 0) {
- _value = UNLIMITED;
+ _value = UNLIMITED();
_max_concurrency = 0;
} else {
_value = butil::string_printf("%d", max_concurrency);
@@ -83,7 +80,7 @@ void AdaptiveMaxConcurrency::operator=(const
butil::StringPiece& value) {
void AdaptiveMaxConcurrency::operator=(int max_concurrency) {
if (max_concurrency <= 0) {
- _value = UNLIMITED;
+ _value = UNLIMITED();
_max_concurrency = 0;
} else {
_value = butil::string_printf("%d", max_concurrency);
@@ -105,9 +102,9 @@ void AdaptiveMaxConcurrency::operator=(const
TimeoutConcurrencyConf& value) {
const std::string& AdaptiveMaxConcurrency::type() const {
if (_max_concurrency > 0) {
- return CONSTANT;
+ return CONSTANT();
} else if (_max_concurrency == 0) {
- return UNLIMITED;
+ return UNLIMITED();
} else {
return _value;
}
diff --git a/src/brpc/adaptive_max_concurrency.h
b/src/brpc/adaptive_max_concurrency.h
index c90def61..2bdcd907 100644
--- a/src/brpc/adaptive_max_concurrency.h
+++ b/src/brpc/adaptive_max_concurrency.h
@@ -65,9 +65,17 @@ public:
// "unlimited", "constant" or "user-defined"
const std::string& type() const;
- // Get strings filled with "unlimited" and "constant"
- static const std::string UNLIMITED;// = "unlimited";
- static const std::string CONSTANT;// = "constant";
+ // Use Meyers' Singleton to avoid static initialization order fiasco:
+ // global AdaptiveMaxConcurrency objects in other translation units may
+ // depend on these strings during their construction.
+ 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;
+ }
void SetConcurrencyLimiter(ConcurrencyLimiter* cl) { _cl = cl; }
diff --git a/src/brpc/policy/constant_concurrency_limiter.cpp
b/src/brpc/policy/constant_concurrency_limiter.cpp
index 425c5f34..7d73e2ec 100644
--- a/src/brpc/policy/constant_concurrency_limiter.cpp
+++ b/src/brpc/policy/constant_concurrency_limiter.cpp
@@ -43,7 +43,7 @@ int ConstantConcurrencyLimiter::ResetMaxConcurrency(
ConstantConcurrencyLimiter*
ConstantConcurrencyLimiter::New(const AdaptiveMaxConcurrency& amc) const {
- CHECK_EQ(amc.type(), AdaptiveMaxConcurrency::CONSTANT);
+ CHECK_EQ(amc.type(), AdaptiveMaxConcurrency::CONSTANT());
return new ConstantConcurrencyLimiter(static_cast<int>(amc));
}
diff --git a/src/brpc/server.cpp b/src/brpc/server.cpp
index 3a5da7b7..935e5f1b 100644
--- a/src/brpc/server.cpp
+++ b/src/brpc/server.cpp
@@ -754,7 +754,7 @@ static int get_port_from_fd(int fd) {
bool Server::CreateConcurrencyLimiter(const AdaptiveMaxConcurrency& amc,
ConcurrencyLimiter** out) {
- if (amc.type() == AdaptiveMaxConcurrency::UNLIMITED) {
+ if (amc.type() == AdaptiveMaxConcurrency::UNLIMITED()) {
*out = NULL;
return true;
}
@@ -1075,7 +1075,7 @@ int Server::StartInternal(const butil::EndPoint& endpoint,
it->second.status->SetConcurrencyLimiter(NULL);
} else {
const AdaptiveMaxConcurrency* amc = &it->second.max_concurrency;
- if (amc->type() == AdaptiveMaxConcurrency::UNLIMITED) {
+ if (amc->type() == AdaptiveMaxConcurrency::UNLIMITED()) {
amc = &_options.method_max_concurrency;
}
ConcurrencyLimiter* cl = NULL;
diff --git a/src/brpc/server.h b/src/brpc/server.h
index 9f69a834..4fbe304f 100644
--- a/src/brpc/server.h
+++ b/src/brpc/server.h
@@ -717,7 +717,7 @@ friend class Controller;
int SetServiceMaxConcurrency(T* service) {
if (NULL != service && NULL != service->_status) {
const AdaptiveMaxConcurrency* amc = &service->_max_concurrency;
- if (amc->type() == AdaptiveMaxConcurrency::UNLIMITED) {
+ if (amc->type() == AdaptiveMaxConcurrency::UNLIMITED()) {
amc = &_options.method_max_concurrency;
}
ConcurrencyLimiter* cl = NULL;
diff --git a/test/brpc_adaptive_class_unittest.cpp
b/test/brpc_adaptive_class_unittest.cpp
index c0d76c00..18128f2a 100644
--- a/test/brpc_adaptive_class_unittest.cpp
+++ b/test/brpc_adaptive_class_unittest.cpp
@@ -31,13 +31,13 @@ const std::string kPooled = "PoOled";
TEST(AdaptiveMaxConcurrencyTest, ShouldConvertCorrectly) {
brpc::AdaptiveMaxConcurrency amc(0);
- EXPECT_EQ(brpc::AdaptiveMaxConcurrency::UNLIMITED, amc.type());
- EXPECT_EQ(brpc::AdaptiveMaxConcurrency::UNLIMITED, amc.value());
+ EXPECT_EQ(brpc::AdaptiveMaxConcurrency::UNLIMITED(), amc.type());
+ EXPECT_EQ(brpc::AdaptiveMaxConcurrency::UNLIMITED(), amc.value());
EXPECT_EQ(0, int(amc));
- EXPECT_TRUE(amc == brpc::AdaptiveMaxConcurrency::UNLIMITED);
+ EXPECT_TRUE(amc == brpc::AdaptiveMaxConcurrency::UNLIMITED());
amc = 10;
- EXPECT_EQ(brpc::AdaptiveMaxConcurrency::CONSTANT, amc.type());
+ EXPECT_EQ(brpc::AdaptiveMaxConcurrency::CONSTANT(), amc.type());
EXPECT_EQ("10", amc.value());
EXPECT_EQ(10, int(amc));
EXPECT_EQ(amc, "10");
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
