Copilot commented on code in PR #3595:
URL: https://github.com/apache/brpc/pull/3595#discussion_r4237267945
##########
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.
Review Comment:
This new failure cleanup makes the existing caller in
`src/brpc/partition_channel.cpp:313-317` unsafe: it uses the default
`OWNS_CHANNEL` option and then executes `delete pchan` when `AddChannel`
returns `-1`. Any failure here that reaches `SubChannel::BeforeRecycle` now
deletes `pchan` first, so that handler double-frees it. Update the caller as
part of this ownership-contract change (and ensure it can distinguish the
duplicate case, where ownership is intentionally not consumed).
--
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]
