yx9o commented on code in PR #11353:
URL: https://github.com/apache/rocketmq/pull/11353#discussion_r4475936709
##########
broker/src/main/java/org/apache/rocketmq/broker/lite/LiteSubscriptionRegistryImpl.java:
##########
@@ -338,16 +338,29 @@ protected void excludeClientByLmqName(String newClientId,
String group, String l
.collect(Collectors.toList());
toRemove.forEach(clientGroup -> {
+ // Capture the channel before the subscription removal so the
conditional
+ // remove below only reclaims the channel observed here, never one
that a
+ // concurrent re-subscribe may have registered in the meantime.
+ Channel evictedChannel = clientChannels.get(clientGroup.clientId);
+ boolean subscriptionRemoved = false;
LiteSubscription liteSubscription =
client2Subscription.get(clientGroup.clientId);
if (liteSubscription != null) {
liteSubscription.removeLmq(lmqName);
// remove client if no more liteTopic
if (liteSubscription.getLmqSet().isEmpty()) {
client2Subscription.remove(clientGroup.clientId);
+ subscriptionRemoved = true;
}
}
exclusiveEvictionTombstones.add(clientGroup.clientId, lmqName);
notifyUnsubscribeLite(clientGroup.clientId, clientGroup.group,
lmqName);
+ if (subscriptionRemoved) {
+ // The eviction dropped the client's last liteTopic, so the
channel entry must be
+ // reclaimed as well: the expiry cleaner only walks
client2Subscription and could
+ // otherwise never remove it, leaking channels under
exclusive-mode client churn.
+ // Conditional remove keeps a channel registered by a
concurrent re-subscribe intact.
+ clientChannels.remove(clientGroup.clientId, evictedChannel);
Review Comment:
Could this removal also account for a concurrent re-subscription on the same
channel? The client may subscribe to another LiteTopic before this line runs,
but `remove(clientId, evictedChannel)` still succeeds because the channel is
unchanged. The active subscription then loses its notification channel.
--
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]
