chenBright commented on code in PR #3595:
URL: https://github.com/apache/brpc/pull/3595#discussion_r4242271120
##########
src/brpc/selective_channel.cpp:
##########
@@ -197,19 +201,33 @@ int ChannelBalancer::AddChannel(ChannelBase* sub_channel,
options.health_check_interval_s = FLAGS_channel_check_interval;
if (Socket::Create(options, &sock_id) != 0) {
Review Comment:
`VersionedRefWithId::Create()` can return when `get_resource()` fails,
before entering `OnCreated()`. No recycling callback runs in that case.
Removing delete `sub_chan` therefore introduces a wrapper-object leak under
both ownership modes. The removed PartitionChannel cleanup also leaves that
caller without a fallback.
##########
src/brpc/socket.cpp:
##########
@@ -722,7 +722,10 @@ int Socket::Create(const SocketOptions& options, SocketId*
id) {
int Socket::OnCreated(const SocketOptions& options) {
if (_io_event.Init((void*)id()) != 0) {
LOG(ERROR) << "Fail to init IOEvent";
- SetFailed(ENOMEM, "%s", "Fail to init IOEvent");
+ // VersionedRefWithId::Create() performs the failure transition after
+ // OnCreated() returns an error; attach the user before returning so
+ // recycling invokes SocketUser::BeforeRecycle.
+ _user = options.user;
Review Comment:
There are two high-priority issues:
1. When `IOEvent::Init()` fails, assigning `_user `causes recycling to
invoke `EpollOutRequest::BeforeRecycle()`, which deletes the request. After
`Create()` returns, `Socket::Connect()` deletes the same request again at line
1353.
```c++
if (Create(options, &connect_id) != 0) {
LOG(FATAL) << "Fail to create Socket";
delete req;
return -1;
}
```
2. `Acceptor::StartAccept()` holds `_map_mutex` while calling
`Socket::Create()`. The newly enabled failure callback synchronously enters
`Acceptor::BeforeRecycle()`, which attempts to acquire the same non-recursive
mutex. This prevents startup from returning an error.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
