This is an automated email from the ASF dual-hosted git repository.
guangmingchen 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 f6572570 fix memory leak issue #2871 (#2872)
f6572570 is described below
commit f65725709dba5e6c82abaa0cf1d5c9a446d5b570
Author: w-gc <[email protected]>
AuthorDate: Wed Jan 29 23:20:38 2025 +0800
fix memory leak issue #2871 (#2872)
* fix memory leak issue
* add comments
* refine
* refine
* fix
* continue to fix
---
src/brpc/server.cpp | 59 +++++++++++++++++++++++++++-----
test/brpc_channel_unittest.cpp | 4 +++
test/brpc_http_rpc_protocol_unittest.cpp | 5 ++-
3 files changed, 58 insertions(+), 10 deletions(-)
diff --git a/src/brpc/server.cpp b/src/brpc/server.cpp
index ade8e274..2da703ef 100644
--- a/src/brpc/server.cpp
+++ b/src/brpc/server.cpp
@@ -151,7 +151,7 @@ ServerOptions::ServerOptions()
, rtmp_service(NULL)
, redis_service(NULL)
, bthread_tag(BTHREAD_TAG_DEFAULT)
- , rpc_pb_message_factory(new DefaultRpcPBMessageFactory())
+ , rpc_pb_message_factory(NULL)
, ignore_eovercrowded(false) {
if (s_ncore > 0) {
num_threads = s_ncore + 1;
@@ -424,7 +424,7 @@ Server::Server(ProfilerLinker)
, _eps_bvar(&_nerror_bvar)
, _concurrency(0)
, _concurrency_bvar(cast_no_barrier_int, &_concurrency)
- ,_has_progressive_read_method(false) {
+ , _has_progressive_read_method(false) {
BAIDU_CASSERT(offsetof(Server, _concurrency) % 64 == 0,
Server_concurrency_must_be_aligned_by_cacheline);
}
@@ -782,6 +782,52 @@ static bool OptionsAvailableOverRdma(const ServerOptions*
opt) {
static AdaptiveMaxConcurrency g_default_max_concurrency_of_method(0);
static bool g_default_ignore_eovercrowded(false);
+inline void copy_and_fill_server_options(ServerOptions& dst, const
ServerOptions& src) {
+// follow Server::~Server()
+#define FREE_PTR_IF_NOT_REUSED(ptr) \
+ if (dst.ptr != src.ptr) { \
+ delete dst.ptr; \
+ dst.ptr = NULL; \
+ }
+
+ if (&dst != &src) {
+ FREE_PTR_IF_NOT_REUSED(nshead_service);
+
+ #ifdef ENABLE_THRIFT_FRAMED_PROTOCOL
+ FREE_PTR_IF_NOT_REUSED(thrift_service);
+ #endif
+
+ FREE_PTR_IF_NOT_REUSED(baidu_master_service);
+ FREE_PTR_IF_NOT_REUSED(http_master_service);
+ FREE_PTR_IF_NOT_REUSED(rpc_pb_message_factory);
+
+ if (dst.pid_file != src.pid_file && !dst.pid_file.empty()) {
+ unlink(dst.pid_file.c_str());
+ }
+
+ if (dst.server_owns_auth) {
+ FREE_PTR_IF_NOT_REUSED(auth);
+ }
+
+ if (dst.server_owns_interceptor) {
+ FREE_PTR_IF_NOT_REUSED(interceptor);
+ }
+
+ FREE_PTR_IF_NOT_REUSED(redis_service);
+
+ // copy data members directly
+ dst = src;
+ }
+#undef FREE_PTR_IF_NOT_REUSED
+
+ // Create the resource if:
+ // 1. `dst` copied from user and user forgot to create
+ // 2. `dst` created by our
+ if (!dst.rpc_pb_message_factory) {
+ dst.rpc_pb_message_factory = new DefaultRpcPBMessageFactory();
+ }
+}
+
int Server::StartInternal(const butil::EndPoint& endpoint,
const PortRange& port_range,
const ServerOptions *opt) {
@@ -813,13 +859,8 @@ int Server::StartInternal(const butil::EndPoint& endpoint,
}
return -1;
}
- if (opt) {
- _options = *opt;
- } else {
- // Always reset to default options explicitly since `_options'
- // may be the options for the last run or even bad options
- _options = ServerOptions();
- }
+
+ copy_and_fill_server_options(_options, opt ? *opt : ServerOptions());
if (!_options.h2_settings.IsValid(true/*log_error*/)) {
LOG(ERROR) << "Invalid h2_settings";
diff --git a/test/brpc_channel_unittest.cpp b/test/brpc_channel_unittest.cpp
index 43d0ab7f..93a4d3bf 100644
--- a/test/brpc_channel_unittest.cpp
+++ b/test/brpc_channel_unittest.cpp
@@ -201,6 +201,10 @@ protected:
ChannelTest()
: _ep(butil::IP_ANY, 8787)
, _close_fd_once(false) {
+ if (!_dummy.options().rpc_pb_message_factory) {
+ _dummy._options.rpc_pb_message_factory = new
brpc::DefaultRpcPBMessageFactory();
+ }
+
pthread_once(®ister_mock_protocol, register_protocol);
const brpc::InputMessageHandler pairs[] = {
{ brpc::policy::ParseRpcMessage,
diff --git a/test/brpc_http_rpc_protocol_unittest.cpp
b/test/brpc_http_rpc_protocol_unittest.cpp
index d2dcf972..cf23debd 100644
--- a/test/brpc_http_rpc_protocol_unittest.cpp
+++ b/test/brpc_http_rpc_protocol_unittest.cpp
@@ -132,7 +132,10 @@ protected:
// Hack: Regard `_server' as running
_server._status = brpc::Server::RUNNING;
_server._options.auth = &_auth;
-
+ if (!_server._options.rpc_pb_message_factory) {
+ _server._options.rpc_pb_message_factory = new
brpc::DefaultRpcPBMessageFactory();
+ }
+
EXPECT_EQ(0, pipe(_pipe_fds));
brpc::SocketId id;
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
