commons-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Chen (Jira)" <j...@apache.org>
Subject [jira] [Comment Edited] (COLLECTIONS-663) Unexpected ConcurrentModificationException when altering Collection of a MultiValuedMap
Date Wed, 06 Nov 2019 06:37:00 GMT

    [ https://issues.apache.org/jira/browse/COLLECTIONS-663?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16968061#comment-16968061
] 

Chen edited comment on COLLECTIONS-663 at 11/6/19 6:36 AM:
-----------------------------------------------------------

Hello. [~chris333]

Thank you for your response.

I think the representation of Multimap is \{1: A, 1: B, 2: C}. So when A and B are deleted,
only \{2: C} is left. And asMap() method  returns a map in the form of {1:[A, B], 2:[C]}.
In this map, {1:[], 2:[C]} is left when A and B are deleted.

If you agree with what I say above, the cause of your problem is not AbstractMultiMap ValueIterator.remove().

The reason is as follow:
{code:java}
// AbstractMultiValuedMap AsMap entrySet() AsMapEntrySet AsMapEntrySetIterator next()
        class AsMapEntrySetIterator extends AbstractIteratorDecorator<Map.Entry<K, Collection<V>>>
{                           
            AsMapEntrySetIterator(final Iterator<Map.Entry<K, Collection<V>>>
iterator) {
                super(iterator);
            }            
           @Override
            public Map.Entry<K, Collection<V>> next() {
                final Map.Entry<K, Collection<V>> entry = super.next();
                final K key = entry.getKey();
                final Collection<V> value = entry.getValue();
                return new UnmodifiableMapEntry<>(key, value);
                //return new UnmodifiableMapEntry<>(key, wrappedCollection(key));
            }
        }
{code}
I think wrappedCollection(key) should not be used。 It willl show as \{1: A, 1: B, 2: C}
when wrappedCollection(key) is used, the asMap() method does not make sense.

And this modification will also avoid your problem. You also can traverse the MultiMap by
mapIterator() method.

Thank you for your listening!


was (Author: guoping1):
Hello. [~chris333]

Thank you for your response.

I think the representation of Multimap is \{1: A, 1: B, 2: C}. So when A and B are deleted,
only \{2: C} is left. And asMap() method  returns a map in the form of \{1:[A, B], 2:[C]}.
In this map, \{1:[], 2:[C]} is left when A and B are deleted.

If you agree with what I say above, the cause of your problem is not AbstractMultiMap ValueIterator.remove().

The reason is as follow:
{code:java}
// AbstractMultiValuedMap AsMap entrySet() AsMapEntrySet AsMapEntrySetIterator next()
        class AsMapEntrySetIterator extends AbstractIteratorDecorator<Map.Entry<K, Collection<V>>>
{                           
            AsMapEntrySetIterator(final Iterator<Map.Entry<K, Collection<V>>>
iterator) {
                super(iterator);
            }            
           @Override
            public Map.Entry<K, Collection<V>> next() {
                final Map.Entry<K, Collection<V>> entry = super.next();
                final K key = entry.getKey();
                final Collection<V> value = entry.getValue();
                return new UnmodifiableMapEntry<>(key, value);
                //return new UnmodifiableMapEntry<>(key, wrappedCollection(key));
            }
        }
{code}
I think wrappedCollection(key) should not be used。 It willl be shown as \{1: A, 1: B, 2:
C} when wrappedCollection(key) is used, the asMap() method does not make sense.

And this modification will also avoid your problem. You also can traverse the MultiMap by
mapIterator() method.

Thank you for your listening!

> Unexpected ConcurrentModificationException when altering Collection of a MultiValuedMap
> ---------------------------------------------------------------------------------------
>
>                 Key: COLLECTIONS-663
>                 URL: https://issues.apache.org/jira/browse/COLLECTIONS-663
>             Project: Commons Collections
>          Issue Type: Bug
>            Reporter: Christophe Schmaltz
>            Assignee: Bruno P. Kinoshita
>            Priority: Trivial
>
> Testcase:
> {code}	@Test
> 	public void test() {
> 		MultiValuedMap<Integer, Integer> multiMap = new HashSetValuedHashMap<>();
> 		multiMap.put(1, 10);
> 		multiMap.put(2, 20);
> 		for (Collection<Integer> innerCollection : multiMap.asMap().values()) {
> 			for (Iterator<Integer> iterator = innerCollection.iterator(); iterator.hasNext();)
{
> 				Integer i = iterator.next();
> 				iterator.remove(); // only the innerCollection is altered
> 			}
> 			// innerCollection.add(6); // adding stuff back should also work...
> 		}
> 	}{code}
> This test unexpectedly throws a ConcurrentModificationException.
> The issue is that when calling {{iterator.remove()}} the {{AbstractMultiValuedMap.ValuesIterator}}
detects that the Collection is empty and calls {{AbstractMultiValuedMap.this.remove(key);}}.
> It may be better if the iterator of the inner collection had a reference on the iterator
if the outer map and called {{containerIterator.remove()}} instead.
> *Note:* this solution would again present issues if the user tries to add new elements
in this now empty collection (which was removed from the parent).
> In the current state, it is quite unclear why an exception is thrown, without debugging
the code. 



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

Mime
View raw message