w-gc commented on code in PR #2872:
URL: https://github.com/apache/brpc/pull/2872#discussion_r1921726312


##########
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:
   有的,有点防不胜防的意味。
   例如`brpc_channel_unittest.cpp:260`:
   ```c++
           brpc::RpcPBMessages* messages =
               ts->_dummy.options().rpc_pb_message_factory->Get(ts->_svc, 
*method);
   ```
   `_dummy`是一个`brpc::Server`对象,其初始化后并没有手动创建rpc_pb_message_factory,最后导致crush。
   
   
如果将‘Server里赋的默认值’删除掉,那么所有类似于`_dummy`的用法都是需要修改的,这样是否会破坏兼容性?此处不确定这样的破坏是否可接受,故保留了在Server里赋的默认值。



-- 
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

Reply via email to