[
https://issues.apache.org/jira/browse/GEODE-8195?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17144989#comment-17144989
]
Bruce J Schuchardt edited comment on GEODE-8195 at 6/25/20, 2:57 PM:
---------------------------------------------------------------------
We do need to remove the successfully transmitted LocatorJoinMessage from the
collection or it will be sent again and again and again. That's the point of
the removal - it's been transmitted and doesn't need to be sent again.
A {color:blue}{{for}}{color} loop in the form {color:blue}{{for (JoinMessage j:
joinMessages)}}{color} implicitly uses an {color:blue}{{Iterator}}{color} to
traverse the collection. The only way to safely remove a value from the
collection is to use {color:blue}{{Iterator.remove()}}{color} method. From the
javadoc for this method:
_The behavior of an iterator is unspecified if the underlying collection is
modified while the iteration is in progress in any way other than by calling
this method._
was (Author: bschuchardt):
We do need to remove the successfully transmitted LocatorJoinMessage from the
collection or it will be sent again and again and again. That's the point of
the removal - it's been transmitted and doesn't need to be sent again.
A {{for}} loop in the form {{for (JoinMessage j: joinMessages}} implicitly uses
an {{Iterator}} to traverse the collection. The only way to safely remove a
value from the collection is to use {{Iterator.remove()}} method. From the
javadoc for this method:
_The behavior of an iterator is unspecified if the underlying collection is
modified while the iteration is in progress in any way other than by calling
this method._
> ConcurrentModificationException from
> LocatorMembershipListenerImpl$DistributeLocatorsRunnable.run
> -------------------------------------------------------------------------------------------------
>
> Key: GEODE-8195
> URL: https://issues.apache.org/jira/browse/GEODE-8195
> Project: Geode
> Issue Type: Bug
> Components: membership
> Reporter: Bill Burcham
> Assignee: Bruce J Schuchardt
> Priority: Major
>
> this WAN code in
> {{LocatorMembershipListenerImpl$DistributeLocatorsRunnable.run}}:
> {code}
> Set<LocatorJoinMessage> joinMessages = entry.getValue();
> for (LocatorJoinMessage locatorJoinMessage : joinMessages) {
> if (retryMessage(targetLocator, locatorJoinMessage, attempt)) {
> joinMessages.remove(locatorJoinMessage);
> } else {
> {code}
> modifies the {{joinMessages}} set as it is iterating over the set, resulting
> in a {{ConcurrentModificationException}}.
> This bug will cause (inter-site) notification of locators (of the presence of
> a new locator) to fail early if retry is necessary. If we have to retry
> notifying any locator, and we succeed, we’ll throw the
> {{ConcurrentModificationException}} and stop trying to notify any of the
> other locators. See the _Discovery For Multi-Site Systems_ section of the
> [Overview of Multi-Site
> Caching|https://geode.apache.org/docs/guide/14/topologies_and_comm/topology_concepts/multisite_overview.html]
> documentation for an overview of the locator's role in WAN.
> Here is a scratch file that illustrates the issue, throwing
> {{ConcurrentModificationException}}:
> {code}
> import java.util.HashSet;
> import java.util.Set;
> class Scratch {
> public static void main(String[] args) {
> final Set<String> joinMessages = new HashSet<>();
> joinMessages.add("one");
> joinMessages.add("two");
> for( final String entry:joinMessages ) {
> if (entry.equals("one"))
> joinMessages.remove(entry);
> }
> }
> }
> {code}
> From looking at the Geode code, {{joinMessages}} is not used outside the loop
> so there is no need to modify it at all—I think we can simply remove this
> line:
> {code}
> joinMessages.remove(locatorJoinMessage);
> {code}
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
