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:28: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:27 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 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.

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 is return 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 the problem is not AbstractMultiMap ValueIterator.remove().

The reasons are as follows:
{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。if it use wrappedCollection(key), it willl
be shown as \{1: A, 1: B, 2: C}. the asMap() method does not make sense. And this modification
will also avoid your problem.

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