two-headBoy commented on code in PR #3595:
URL: https://github.com/apache/brpc/pull/3595#discussion_r4246993883
##########
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) {
- delete sub_chan;
+ // On every creation-failure path Socket recycles synchronously and
runs
+ // SubChannel::BeforeRecycle, which deletes chan iff OWNS_CHANNEL and
+ // then deletes sub_chan. Do NOT delete here: that double-frees.
LOG(ERROR) << "Fail to create fake socket for sub channel";
return -1;
}
SocketUniquePtr ptr;
int rc = Socket::AddressFailedAsWell(sock_id, &ptr);
if (rc < 0) {
LOG(ERROR) << "Fail to address SocketId=" << sock_id;
+ // The fake Socket was created (holding the HC-related reference) but
+ // cannot be addressed. Recycle it so that sub_chan, and sub_channel
+ // when OWNS_CHANNEL, are deleted in SubChannel::BeforeRecycle.
+ SocketUniquePtr fail_ptr;
+ if (Socket::AddressFailedAsWell(sock_id, &fail_ptr) >= 0) {
Review Comment:
Correct — that second`AddressFailedAsWell` can never succeed (`Create` just
returned 0 holding the additional reference; once the first call returns -1 the
slot is version-mismatched, so the repeat also returns -1 and`fail_ptr` is
never set). Removed the dead block and its comment.`rc < 0` means the slot
version advanced past`id_ver+1` , i.e. the socket was already`SetFailed`
/recycled, so`BeforeRecycle` has already cleaned up`sub_chan` ; the branch now
just logs and returns.
--
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]
