MOB-22620: Fix concurrent modification exception during session termination #147
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Problem: `notifyTerminationAndRemoveStatusListeners()` iterates over all the connection status listeners to notify disconnection and un-registers them. Since the de-registration process involves removing the listeners from the set, iterating over the same list for removal leads to ConcurrentModificationException and not all listeners are notified of the error. This results in the connecting icon spinning forever. Solution: Iterate for de-registration over a copy of the listeners to prevent the exception and propogate the error.
praveek
reviewed
Jan 21, 2025
|
|
||
| // Unregister all the listeners. Operate on a copy of the listeners as | ||
| // unregisterStatusListener modifies the listeners set. | ||
| final Set<AssuranceSessionStatusListener> listenersCopy = |
There was a problem hiding this comment.
How about sessionStatusListeners.clear() ?
There was a problem hiding this comment.
Yes! Clearing here is better. Will patch.
cdhoffmann
approved these changes
Jan 23, 2025
praveek
approved these changes
Jan 27, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Problem
notifyTerminationAndRemoveStatusListeners()iterates over all the connection status listeners to notify disconnection and un-registers them. Since the de-registration process involves removing the listeners from the set, iterating over the same list for removal leads to ConcurrentModificationException and not all listeners are notified of the error. This results in the connecting icon spinning forever.This situation only occurs for terminal errors/ non retryable errors so it does not affect the normal workflow.
Solution
Iterate for de-registration over a copy of the listeners to prevent the exception and propagate the error.
Related Issue
MOB-22620
Motivation and Context
notifyTerminationAndRemoveStatusListeners()iterates over all the connection status listeners to notify disconnection and un-registers them. Since the de-registration process involves removing the listeners from the set, iterating over the same list for removal leads to ConcurrentModificationException and not all listeners are notified of the error. This results in the connecting icon spinning forever.As a solution, iterate for de-registration over a copy of the listeners to prevent the exception and propagate the error.
How Has This Been Tested?
Screenshots (if appropriate):
Types of changes
Checklist: