w-gc commented on code in PR #2872:
URL: https://github.com/apache/brpc/pull/2872#discussion_r1926521633
##########
src/brpc/server.cpp:
##########
@@ -813,14 +857,26 @@ int Server::StartInternal(const butil::EndPoint& endpoint,
}
return -1;
}
+
if (opt) {
- _options = *opt;
+ copy_server_option(_options, *opt);
} else {
+ // Don't forget to release `rpc_pb_message_factory` before overwriting
`_options`
+ delete _options.rpc_pb_message_factory;
+ _options.rpc_pb_message_factory = NULL;
+
// Always reset to default options explicitly since `_options'
// may be the options for the last run or even bad options
_options = ServerOptions();
}
+ // Create the resource if:
+ // 1. `_options` copied from user and user forgot to create
+ // 2. `_options` created by our
+ if (!_options.rpc_pb_message_factory) {
+ _options.rpc_pb_message_factory = new DefaultRpcPBMessageFactory();
+ }
Review Comment:
这个我尝试了不太好改，可能是我对这个Server的使用流程不太熟悉，不太清楚UT里有server但没有启动情况下怎么使用rpc_pb_message_factory：
```c++
const google::protobuf::MethodDescriptor* method =
ts->_svc.descriptor()->FindMethodByName(req_meta.method_name());
// 假启动，只为更新options
if (!ts->_dummy.options().rpc_pb_message_factory) {
ts->_dummy.Start(ts->_ep, &ts->_dummy.options());
ts->_dummy.Stop(0);
}
brpc::RpcPBMessages* messages =
ts->_dummy.options().rpc_pb_message_factory->Get(ts->_svc, *method);
```
还是
```c++
const google::protobuf::MethodDescriptor* method =
ts->_svc.descriptor()->FindMethodByName(req_meta.method_name());
auto options = ts->_dummy.options();
if (!options.rpc_pb_message_factory) {
options.rpc_pb_message_factory = new brpc::DefaultRpcPBMessageFactory();
}
brpc::RpcPBMessages* messages =
options.rpc_pb_message_factory->Get(ts->_svc, *method);
```
我建议，我这个PR先不改，后续你们来改，或者单独起个PR解决这个问题？毕竟这个语义的变更是会影响到用户的。
--
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: dev-unsubscr...@brpc.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@brpc.apache.org
For additional commands, e-mail: dev-h...@brpc.apache.org
