two-headBoy commented on code in PR #3595:
URL: https://github.com/apache/brpc/pull/3595#discussion_r4237219097
##########
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;
+ // Socket recycles itself on failure, already running
+ // SubChannel::BeforeRecycle (deletes chan iff OWNS, then sub_chan).
+ // Do NOT delete sub_chan/chan here: that double-frees.
Review Comment:
Fixed by attaching`_user` in the single`OnCreated` failure point that
precedes it (`_io_event.Init` ), so the synchronous recycle runs`BeforeRecycle`
on every creation-failure path. Chosen deliberately over a per-`SubChannel`
observation flag: this adds zero per-object bytes (sockets are allocated at
high volume — one per connection plus per-server fake sockets in load
balancers), keeps the change confined to the one leaking branch , and hardens
the cleanup contract for all`SocketUser` s rather than patching schan
alone.`SubChannel` stays unchanged in size;`selective_channel` simply stops
manually deleting on`Socket::Create` failure.
--
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]
