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 f6203937 fix(channel): Reject reinitialization to keep SocketMap
references balanced (#3433)
f6203937 is described below
commit f6203937a983bc5507165883ea09febb42d63773
Author: darion-yaphet <[email protected]>
AuthorDate: Tue Aug 25 19:51:03 2026 +0800
fix(channel): Reject reinitialization to keep SocketMap references balanced
(#3433)
Reject re-initialization once Channel::Init() has succeeded. This ensures
a Channel instance only inserts into SocketMap at most once and preserves
its options and signature intact, guaranteeing that ~Channel() always
balances the insertion without requiring extra state tracking. Failed inits
before the first successful initialization can still be retried.
---
docs/cn/client.md | 3 +
docs/en/client.md | 6 ++
src/brpc/channel.cpp | 16 +++
src/brpc/channel.h | 3 +
test/brpc_channel_unittest.cpp | 222 +++++++++++++++++++++++++++++++----------
5 files changed, 196 insertions(+), 54 deletions(-)
diff --git a/docs/cn/client.md b/docs/cn/client.md
index a3531147..e659403c 100755
--- a/docs/cn/client.md
+++ b/docs/cn/client.md
@@ -7,6 +7,7 @@
Echo的[client端代码](https://github.com/apache/brpc/blob/master/example/echo
# 事实速查
- Channel.Init()是线程不安全的。
+- 一个Channel只能成功初始化一次。Init()失败后可以重试。
- Channel.CallMethod()是线程安全的，一个Channel可以被所有线程同时使用。
- Channel可以分配在栈上。
- Channel在发送异步请求后可以析构。
@@ -32,6 +33,8 @@ channel.Init(..., &options);
```
注意Channel不会修改options，Init结束后不会再访问options。所以options一般就像上面代码中那样放栈上。Channel.options()可以获得channel在使用的所有选项。
+Init失败后可以重试；一旦成功，Channel的目标和选项即固定，之后的所有Init调用都会返回-1。需要使用不同的目标或配置时，请新建一个Channel。
+
Init函数分为连接一台服务器和连接服务集群。
# 连接一台服务器
diff --git a/docs/en/client.md b/docs/en/client.md
index 087c39b5..266eb902 100644
--- a/docs/en/client.md
+++ b/docs/en/client.md
@@ -7,6 +7,8 @@
# Quick facts
- Channel.Init() is not thread-safe.
+- A Channel can be initialized successfully only once. Failed Init() calls may
+ be retried.
- Channel.CallMethod() is thread-safe and a Channel can be used by multiple
threads simultaneously.
- Channel can be put on stack.
- Channel can be destructed just after sending asynchronous request.
@@ -32,6 +34,10 @@ channel.Init(..., &options);
```
Note that Channel neither modifies `options` nor accesses `options` after
completion of Init(), thus options can be put on stack safely as in above code.
Channel.options() gets options being used by the Channel.
+Init() may be retried after a failure. Once it succeeds, the Channel's target
+and options are fixed and every later Init() call returns -1. Create a new
+Channel to use a different target or configuration.
+
Init() can connect one server or a cluster(multiple servers).
# Connect to a server
diff --git a/src/brpc/channel.cpp b/src/brpc/channel.cpp
index 01d86375..ff81e521 100644
--- a/src/brpc/channel.cpp
+++ b/src/brpc/channel.cpp
@@ -254,6 +254,10 @@ int Channel::InitChannelOptions(const ChannelOptions*
options) {
int Channel::Init(const char* server_addr_and_port,
const ChannelOptions* options) {
+ if (_server_id != INVALID_SOCKET_ID || _lb != NULL) {
+ LOG(ERROR) << "Channel=" << this << " has already been initialized";
+ return -1;
+ }
GlobalInitializeOrDie();
butil::EndPoint point;
const AdaptiveProtocolType& ptype = (options ? options->protocol :
_options.protocol);
@@ -287,6 +291,10 @@ int Channel::Init(const char* server_addr_and_port,
int Channel::Init(const char* server_addr, int port,
const ChannelOptions* options) {
+ if (_server_id != INVALID_SOCKET_ID || _lb != NULL) {
+ LOG(ERROR) << "Channel=" << this << " has already been initialized";
+ return -1;
+ }
GlobalInitializeOrDie();
butil::EndPoint point;
const AdaptiveProtocolType& ptype = (options ? options->protocol :
_options.protocol);
@@ -358,6 +366,10 @@ int Channel::InitSingle(const butil::EndPoint&
server_addr_and_port,
const char* raw_server_address,
const ChannelOptions* options,
int raw_port) {
+ if (_server_id != INVALID_SOCKET_ID || _lb != NULL) {
+ LOG(ERROR) << "Channel=" << this << " has already been initialized";
+ return -1;
+ }
GlobalInitializeOrDie();
if (InitChannelOptions(options) != 0) {
return -1;
@@ -410,6 +422,10 @@ int Channel::Init(const char* ns_url,
// Treat ns_url as server_addr_and_port
return Init(ns_url, options);
}
+ if (_server_id != INVALID_SOCKET_ID || _lb != NULL) {
+ LOG(ERROR) << "Channel=" << this << " has already been initialized";
+ return -1;
+ }
GlobalInitializeOrDie();
if (InitChannelOptions(options) != 0) {
return -1;
diff --git a/src/brpc/channel.h b/src/brpc/channel.h
index d13ae52d..47f262f6 100644
--- a/src/brpc/channel.h
+++ b/src/brpc/channel.h
@@ -184,6 +184,9 @@ public:
DISALLOW_COPY_AND_ASSIGN(Channel);
+ // Init() may be retried after failure, but a successful initialization is
+ // final: subsequent calls return -1.
+
// Connect this channel to a single server whose address is given by the
// first parameter. Use default options if `options' is nullptr.
int Init(butil::EndPoint server_addr_and_port, const ChannelOptions*
options);
diff --git a/test/brpc_channel_unittest.cpp b/test/brpc_channel_unittest.cpp
index 997859ef..b6125e26 100644
--- a/test/brpc_channel_unittest.cpp
+++ b/test/brpc_channel_unittest.cpp
@@ -2316,6 +2316,55 @@ TEST_F(ChannelTest, init_as_single_server) {
}
}
+TEST_F(ChannelTest, reject_reinitialization_after_successful_init) {
+ butil::EndPoint first_endpoint;
+ butil::EndPoint second_endpoint;
+ ASSERT_EQ(0, str2endpoint("127.0.0.1:59347", &first_endpoint));
+ ASSERT_EQ(0, str2endpoint("127.0.0.1:59348", &second_endpoint));
+ const brpc::SocketMapKey first_key(first_endpoint);
+ const brpc::SocketMapKey second_key(second_endpoint);
+
+ {
+ brpc::Channel channel;
+ ASSERT_EQ(0, channel.Init(first_endpoint, NULL));
+ brpc::SocketId id;
+ ASSERT_EQ(0, brpc::SocketMapFind(first_key, &id));
+ ASSERT_EQ(channel._server_id, id);
+ ASSERT_EQ(-1, channel.Init(first_endpoint, NULL));
+ ASSERT_EQ(-1, channel.Init(second_endpoint, NULL));
+ ASSERT_EQ(-1, channel.Init("unknown://unknown", "rr", NULL));
+ }
+
+ brpc::SocketId id;
+ EXPECT_NE(0, brpc::SocketMapFind(first_key, &id));
+ EXPECT_NE(0, brpc::SocketMapFind(second_key, &id));
+}
+
+TEST_F(ChannelTest, retry_init_after_failed_init) {
+ butil::EndPoint endpoint;
+ ASSERT_EQ(0, str2endpoint("127.0.0.1:59349", &endpoint));
+ const brpc::SocketMapKey key(endpoint);
+
+ {
+ brpc::Channel channel;
+ brpc::ChannelOptions invalid_options;
+ invalid_options.client_host = "not a valid client host";
+ ASSERT_EQ(-1, channel.Init(endpoint, &invalid_options));
+ EXPECT_EQ(brpc::INVALID_SOCKET_ID, channel._server_id);
+
+ brpc::ChannelOptions valid_options;
+ ASSERT_EQ(0, channel.Init(endpoint, &valid_options));
+ EXPECT_NE(brpc::INVALID_SOCKET_ID, channel._server_id);
+ EXPECT_EQ(endpoint, channel._server_address);
+ brpc::SocketId id;
+ ASSERT_EQ(0, brpc::SocketMapFind(key, &id));
+ ASSERT_EQ(channel._server_id, id);
+ }
+
+ brpc::SocketId id;
+ EXPECT_NE(0, brpc::SocketMapFind(key, &id));
+}
+
TEST_F(ChannelTest, init_using_unknown_naming_service) {
brpc::Channel channel;
ASSERT_EQ(-1, channel.Init("unknown://unknown", "unknown", nullptr));
@@ -2391,73 +2440,138 @@ TEST_F(ChannelTest, parse_hostname) {
brpc::ChannelOptions opt;
opt.succeed_without_server = false;
opt.protocol = brpc::PROTOCOL_HTTP;
- brpc::Channel channel;
- ASSERT_EQ(-1, channel.Init("", 8888, &opt));
- ASSERT_EQ("", channel._service_name);
- ASSERT_EQ(-1, channel.Init("", &opt));
- ASSERT_EQ("", channel._service_name);
-
- ASSERT_EQ(0, channel.Init("http://127.0.0.1";, 8888, &opt));
- ASSERT_EQ("127.0.0.1:8888", channel._service_name);
- ASSERT_EQ(0, channel.Init("http://127.0.0.1:8888";, &opt));
- ASSERT_EQ("127.0.0.1:8888", channel._service_name);
-
- ASSERT_EQ(0, channel.Init("localhost", 8888, &opt));
- ASSERT_EQ("localhost:8888", channel._service_name);
- ASSERT_EQ(0, channel.Init("localhost:8888", &opt));
- ASSERT_EQ("localhost:8888", channel._service_name);
-
- ASSERT_EQ(0, channel.Init("http://www.baidu.com";, &opt));
- ASSERT_EQ("www.baidu.com", channel._service_name);
- ASSERT_EQ(0, channel.Init("http://www.baidu.com:80";, &opt));
- ASSERT_EQ("www.baidu.com:80", channel._service_name);
- ASSERT_EQ(0, channel.Init("http://www.baidu.com";, 80, &opt));
- ASSERT_EQ("www.baidu.com:80", channel._service_name);
- ASSERT_EQ(0, channel.Init("http://www.baidu.com:8888";, &opt));
- ASSERT_EQ("www.baidu.com:8888", channel._service_name);
- ASSERT_EQ(0, channel.Init("http://www.baidu.com";, 8888, &opt));
- ASSERT_EQ("www.baidu.com:8888", channel._service_name);
- ASSERT_EQ(0, channel.Init("http://www.baidu.com";, "rr", &opt));
- ASSERT_EQ("www.baidu.com", channel._service_name);
- ASSERT_EQ(0, channel.Init("http://www.baidu.com:80";, "rr", &opt));
- ASSERT_EQ("www.baidu.com:80", channel._service_name);
- ASSERT_EQ(0, channel.Init("http://www.baidu.com:8888";, "rr", &opt));
- ASSERT_EQ("www.baidu.com:8888", channel._service_name);
+ {
+ brpc::Channel channel;
+ ASSERT_EQ(-1, channel.Init("", 8888, &opt));
+ ASSERT_EQ("", channel._service_name);
+ }
+ {
+ brpc::Channel channel;
+ ASSERT_EQ(-1, channel.Init("", &opt));
+ ASSERT_EQ("", channel._service_name);
+ }
+
+ {
+ brpc::Channel channel;
+ ASSERT_EQ(0, channel.Init("http://127.0.0.1";, 8888, &opt));
+ ASSERT_EQ("127.0.0.1:8888", channel._service_name);
+ }
+ {
+ brpc::Channel channel;
+ ASSERT_EQ(0, channel.Init("http://127.0.0.1:8888";, &opt));
+ ASSERT_EQ("127.0.0.1:8888", channel._service_name);
+ }
+
+ {
+ brpc::Channel channel;
+ ASSERT_EQ(0, channel.Init("localhost", 8888, &opt));
+ ASSERT_EQ("localhost:8888", channel._service_name);
+ }
+ {
+ brpc::Channel channel;
+ ASSERT_EQ(0, channel.Init("localhost:8888", &opt));
+ ASSERT_EQ("localhost:8888", channel._service_name);
+ }
+
+ {
+ brpc::Channel channel;
+ ASSERT_EQ(0, channel.Init("http://www.baidu.com";, &opt));
+ ASSERT_EQ("www.baidu.com", channel._service_name);
+ }
+ {
+ brpc::Channel channel;
+ ASSERT_EQ(0, channel.Init("http://www.baidu.com:80";, &opt));
+ ASSERT_EQ("www.baidu.com:80", channel._service_name);
+ }
+ {
+ brpc::Channel channel;
+ ASSERT_EQ(0, channel.Init("http://www.baidu.com";, 80, &opt));
+ ASSERT_EQ("www.baidu.com:80", channel._service_name);
+ }
+ {
+ brpc::Channel channel;
+ ASSERT_EQ(0, channel.Init("http://www.baidu.com:8888";, &opt));
+ ASSERT_EQ("www.baidu.com:8888", channel._service_name);
+ }
+ {
+ brpc::Channel channel;
+ ASSERT_EQ(0, channel.Init("http://www.baidu.com";, 8888, &opt));
+ ASSERT_EQ("www.baidu.com:8888", channel._service_name);
+ }
+ {
+ brpc::Channel channel;
+ ASSERT_EQ(0, channel.Init("http://www.baidu.com";, "rr", &opt));
+ ASSERT_EQ("www.baidu.com", channel._service_name);
+ }
+ {
+ brpc::Channel channel;
+ ASSERT_EQ(0, channel.Init("http://www.baidu.com:80";, "rr", &opt));
+ ASSERT_EQ("www.baidu.com:80", channel._service_name);
+ }
+ {
+ brpc::Channel channel;
+ ASSERT_EQ(0, channel.Init("http://www.baidu.com:8888";, "rr", &opt));
+ ASSERT_EQ("www.baidu.com:8888", channel._service_name);
+ }
opt.mutable_ssl_options()->verify.verify_mode =
brpc::VerifyMode::VERIFY_PEER;
opt.mutable_ssl_options()->verify.verify_depth = 1;
opt.mutable_ssl_options()->verify.ca_file_path = "cert1.crt";
- ASSERT_EQ(0, channel.Init("https://www.baidu.com";, &opt));
- ASSERT_EQ("www.baidu.com", channel._service_name);
+ {
+ brpc::Channel channel;
+ ASSERT_EQ(0, channel.Init("https://www.baidu.com";, &opt));
+ ASSERT_EQ("www.baidu.com", channel._service_name);
#if defined(USE_MESALINK) || \
(!defined(OPENSSL_IS_BORINGSSL) && OPENSSL_VERSION_NUMBER < 0x10002000L)
-
ASSERT_TRUE(channel._options.ssl_options().verify.expected_peer_name.empty());
+
ASSERT_TRUE(channel._options.ssl_options().verify.expected_peer_name.empty());
#else
- ASSERT_EQ("www.baidu.com",
- channel._options.ssl_options().verify.expected_peer_name);
+ ASSERT_EQ("www.baidu.com",
+ channel._options.ssl_options().verify.expected_peer_name);
#endif
- ASSERT_EQ(0, channel.Init("https://www.baidu.com:443";, &opt));
- ASSERT_EQ("www.baidu.com:443", channel._service_name);
+ }
+ {
+ brpc::Channel channel;
+ ASSERT_EQ(0, channel.Init("https://www.baidu.com:443";, &opt));
+ ASSERT_EQ("www.baidu.com:443", channel._service_name);
#if defined(USE_MESALINK) || \
(!defined(OPENSSL_IS_BORINGSSL) && OPENSSL_VERSION_NUMBER < 0x10002000L)
-
ASSERT_TRUE(channel._options.ssl_options().verify.expected_peer_name.empty());
+
ASSERT_TRUE(channel._options.ssl_options().verify.expected_peer_name.empty());
#else
- ASSERT_EQ("www.baidu.com",
- channel._options.ssl_options().verify.expected_peer_name);
+ ASSERT_EQ("www.baidu.com",
+ channel._options.ssl_options().verify.expected_peer_name);
#endif
- ASSERT_EQ(0, channel.Init("https://www.baidu.com";, 443, &opt));
- ASSERT_EQ("www.baidu.com:443", channel._service_name);
- ASSERT_EQ(0, channel.Init("https://www.baidu.com:1443";, &opt));
- ASSERT_EQ("www.baidu.com:1443", channel._service_name);
- ASSERT_EQ(0, channel.Init("https://www.baidu.com";, 1443, &opt));
- ASSERT_EQ("www.baidu.com:1443", channel._service_name);
- ASSERT_EQ(0, channel.Init("https://www.baidu.com";, "rr", &opt));
- ASSERT_EQ("www.baidu.com", channel._service_name);
- ASSERT_EQ(0, channel.Init("https://www.baidu.com:443";, "rr", &opt));
- ASSERT_EQ("www.baidu.com:443", channel._service_name);
- ASSERT_EQ(0, channel.Init("https://www.baidu.com:1443";, "rr", &opt));
- ASSERT_EQ("www.baidu.com:1443", channel._service_name);
+ }
+ {
+ brpc::Channel channel;
+ ASSERT_EQ(0, channel.Init("https://www.baidu.com";, 443, &opt));
+ ASSERT_EQ("www.baidu.com:443", channel._service_name);
+ }
+ {
+ brpc::Channel channel;
+ ASSERT_EQ(0, channel.Init("https://www.baidu.com:1443";, &opt));
+ ASSERT_EQ("www.baidu.com:1443", channel._service_name);
+ }
+ {
+ brpc::Channel channel;
+ ASSERT_EQ(0, channel.Init("https://www.baidu.com";, 1443, &opt));
+ ASSERT_EQ("www.baidu.com:1443", channel._service_name);
+ }
+ {
+ brpc::Channel channel;
+ ASSERT_EQ(0, channel.Init("https://www.baidu.com";, "rr", &opt));
+ ASSERT_EQ("www.baidu.com", channel._service_name);
+ }
+ {
+ brpc::Channel channel;
+ ASSERT_EQ(0, channel.Init("https://www.baidu.com:443";, "rr", &opt));
+ ASSERT_EQ("www.baidu.com:443", channel._service_name);
+ }
+ {
+ brpc::Channel channel;
+ ASSERT_EQ(0, channel.Init("https://www.baidu.com:1443";, "rr", &opt));
+ ASSERT_EQ("www.baidu.com:1443", channel._service_name);
+ }
const char *address_list[] = {
"10.127.0.1:1234",
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
