Skip to content

fix: make consumer group map thread-safe and guard NPE when no brokers - #710

Open
yyqdbngt wants to merge 1 commit into
apache:masterfrom
yyqdbngt:fix/consumer-service-npe-and-thread-safety
Open

fix: make consumer group map thread-safe and guard NPE when no brokers#710
yyqdbngt wants to merge 1 commit into
apache:masterfrom
yyqdbngt:fix/consumer-service-npe-and-thread-safety

Conversation

@yyqdbngt

Copy link
Copy Markdown

Motivation

Two robustness fixes in ConsumerServiceImpl:

  1. consumerGroupMap is not thread-safe
    The map is a plain HashMap but is read/written concurrently: queryGroupList/makeGroupListCache mutate it (on request threads), while refreshAllGroup calls consumerGroupMap.clear() without holding the same lock and refreshGroup reads it directly. This can trigger ConcurrentModificationException or lost updates. Changed it to a ConcurrentHashMap.

  2. NPE when the cluster has no brokers
    In makeGroupListCache, subscriptionGroupWrapper starts as null and is only assigned inside the per-broker loop. When the broker table is empty the wrapper stays null, so subscriptionGroupWrapper.getSubscriptionGroupTable() throws a NullPointerException. The empty/null check is now subscriptionGroupWrapper == null || ...isEmpty() and returns early instead.

Verification

mvn compiler:compile passes (BUILD SUCCESS).

Diff

1 file changed, +5 / -2.

- consumerGroupMap is a plain HashMap read/written from request threads
  (queryGroupList/makeGroupListCache) and refreshAllGroup, which clears it
  without holding the lock; switch to ConcurrentHashMap to avoid
  ConcurrentModificationException and lost updates
- makeGroupListCache dereferences subscriptionGroupWrapper without a null
  check; when the cluster has no brokers the wrapper stays null and
  getSubscriptionGroupTable() throws NPE, guard it and return early
@yyqdbngt
yyqdbngt force-pushed the fix/consumer-service-npe-and-thread-safety branch from 7e82fe1 to d057cdc Compare July 31, 2026 15:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant