From c0aaf32d5823ec9255366b56e2f95df86a8d65d4 Mon Sep 17 00:00:00 2001 From: wangchenguang Date: Tue, 18 Aug 2026 01:34:48 +0800 Subject: [PATCH 1/3] fix(channel): Reject reinitialization to keep SocketMap references balanced 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. --- src/brpc/channel.cpp | 16 +++ test/brpc_channel_unittest.cpp | 213 ++++++++++++++++++++++++--------- 2 files changed, 175 insertions(+), 54 deletions(-) diff --git a/src/brpc/channel.cpp b/src/brpc/channel.cpp index 83fc37b077..0a6e121b28 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/test/brpc_channel_unittest.cpp b/test/brpc_channel_unittest.cpp index 6f4540d6a2..73b04887d1 100644 --- a/test/brpc_channel_unittest.cpp +++ b/test/brpc_channel_unittest.cpp @@ -2316,6 +2316,46 @@ 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)); + + { + brpc::Channel channel; + ASSERT_EQ(0, channel.Init(first_endpoint, NULL)); + 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(brpc::SocketMapKey(first_endpoint), &id)); + EXPECT_NE(0, brpc::SocketMapFind(brpc::SocketMapKey(second_endpoint), &id)); +} + +TEST_F(ChannelTest, retry_init_after_failed_init) { + butil::EndPoint endpoint; + ASSERT_EQ(0, str2endpoint("127.0.0.1:59349", &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; + EXPECT_NE(0, brpc::SocketMapFind(brpc::SocketMapKey(endpoint), &id)); +} + TEST_F(ChannelTest, init_using_unknown_naming_service) { brpc::Channel channel; ASSERT_EQ(-1, channel.Init("unknown://unknown", "unknown", NULL)); @@ -2391,73 +2431,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", From 1de7d65943561415078a38101208f56bc17a90fd Mon Sep 17 00:00:00 2001 From: wangchenguang Date: Tue, 25 Aug 2026 15:57:56 +0800 Subject: [PATCH 2/3] Document Channel initialization as a single-shot operation The implementation and regression tests intentionally reject every Init() call after the first success while permitting retries after failures. Document that contract at the public API and client-guide entry points so users do not infer that a Channel can safely replace its target in place. --- docs/cn/client.md | 3 +++ docs/en/client.md | 6 ++++++ src/brpc/channel.h | 3 +++ 3 files changed, 12 insertions(+) diff --git a/docs/cn/client.md b/docs/cn/client.md index bb16eefb68..1d70280799 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 8da0c6b156..67af7c640c 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.h b/src/brpc/channel.h index 28a17ac8ea..487214c7ba 100644 --- a/src/brpc/channel.h +++ b/src/brpc/channel.h @@ -184,6 +184,9 @@ friend class SelectiveChannel; 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 NULL. int Init(butil::EndPoint server_addr_and_port, const ChannelOptions* options); From 1a4dfa48def7e9f4b075e746a05ae587bd3e4e63 Mon Sep 17 00:00:00 2001 From: wangchenguang Date: Tue, 25 Aug 2026 15:57:56 +0800 Subject: [PATCH 3/3] Make SocketMap cleanup tests validate their lookup keys The reinitialization and retry tests now prove that each lookup key identifies the live channel socket before relying on that same key to verify cleanup after destruction. --- test/brpc_channel_unittest.cpp | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/test/brpc_channel_unittest.cpp b/test/brpc_channel_unittest.cpp index 73b04887d1..a0ddf68f98 100644 --- a/test/brpc_channel_unittest.cpp +++ b/test/brpc_channel_unittest.cpp @@ -2321,23 +2321,29 @@ TEST_F(ChannelTest, reject_reinitialization_after_successful_init) { 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(brpc::SocketMapKey(first_endpoint), &id)); - EXPECT_NE(0, brpc::SocketMapFind(brpc::SocketMapKey(second_endpoint), &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; @@ -2350,10 +2356,13 @@ TEST_F(ChannelTest, retry_init_after_failed_init) { 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(brpc::SocketMapKey(endpoint), &id)); + EXPECT_NE(0, brpc::SocketMapFind(key, &id)); } TEST_F(ChannelTest, init_using_unknown_naming_service) {