You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@commons.apache.org by "Jack, Paul" <pj...@sfaf.org> on 2002/05/08 20:10:14 UTC

RE: [Collections] Bug or Feature? - SequencedHashMap iterators do not throw ConcurrentModificationExceptions

My suggestion would be to have the apache collections mimic the
java.util collections whereever possible, and so treat this as a
bug.  Iterators aren't required to raise ConcurrentModificationExceptions
because in theory, a collection (say, one that's backed by a database)
might not be able to provide that feature.  Something like SequencedHashMap,
which absolutely CAN provide the feature, probably SHOULD provide the 
feature...

Furthermore, I seriously doubt that there's any code out there that relies
on the fact that SequenceHashMap's iterators don't raise
ConcurrentModificationExceptions,
and even if such code exists, its behavior is undefined at this point:  It 
seems we can fix this in a future release without worrying about breaking 
existing code.

Just my 2 cents. 

-Paul

-----Original Message-----
From: Morgan Delagrange [mailto:mdelagra@yahoo.com]
Sent: Wednesday, May 08, 2002 11:09 AM
To: Jakarta Commons Developers
Subject: [Collections] Bug or Feature? - SequencedHashMap iterators do
not throw ConcurrentModificationExceptions


Hi all,

In tracking down a bug in LRUMap, I added a new unit test to testMap() to
check if all our Map Iterators throw ConcurrentModificationExceptions when
the underlying collection is modified.  SequencedHashMap does not.  I know
that it's not absolutely _required_ to handle modifications that way, but
should we classify this as a bug or an unsupported feature?  It seems risky
not to throw a ConcurrentModificationException, because it probably makes
the behaviour of the Iterator indeterminate, and in the case of LRUMap I
think it's leading to an infinite promotion loop.

- Morgan



--
To unsubscribe, e-mail:
<ma...@jakarta.apache.org>
For additional commands, e-mail:
<ma...@jakarta.apache.org>

--
To unsubscribe, e-mail:   <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>


Re: [Collections] Bug or Feature? - SequencedHashMap iterators do not throw ConcurrentModificationExceptions

Posted by Morgan Delagrange <md...@yahoo.com>.
----- Original Message -----
From: "Michael A. Smith" <ma...@apache.org>
To: "Jakarta Commons Developers List" <co...@jakarta.apache.org>;
<mo...@apache.org>
Sent: Wednesday, May 08, 2002 1:54 PM
Subject: RE: [Collections] Bug or Feature? - SequencedHashMap iterators do
not throw ConcurrentModificationExceptions


> On Wed, 8 May 2002, Morgan Delagrange wrote:
> > That's my inclination too.  I'm in a real quandry
> > about how to handle the LRUMap implementation though.
> > If we make those iterators fail-fast, the following
> > operation will produce a
> > ConcurrentModificationException:
> >
> >   Iterator lruMapKeys = lruMap.keySet().iterator();
> >
> >   while (lruMapKeys.hasNext()) {
> >     // the second element fails, because
> >     // calling get(Object) modifies the underlying
> >     // SequencedHashMap
> >     System.out.println(lruMap.get(lruMapKeys.next()));
> >   }
>
> If you want to iterate over the values to print each of them, use
> values().iterator():
>
>   Iterator lruMapValues = lruMap.values().iterator();
>
>   while(lruMapValues.hasNext()) {
>     System.out.println(lruMapValues.next());
>   }
>
> Or, if you need to the key and the value, use entrySet().iterator():
>
>   Iterator lruMapEntries = lruMap.entrySet().iterator();
>
>   while(lruMapEntries.hasNext()) {
>     Map.Entry entry = (Map.Entry)lruMapEntries.next();
>     System.out.println(entry.getValue());
>   }
>
> Both of these use the iterator without touching the map, and thus without
> a concurrent modification.

True, both of those work.  However, by far the most common approach would be
iterating over the keys and getting the values.  With the current
implementation of LRUMap(), that causes problems.  If you try to slide an
LRUMap into an interface that takes any Map, you might get some strangeness.
Either we need an LRUMap that doesn't create an infinite loop under that
circumstance, or we need to throw a ConcurrentModificationException.

> > It's hard to imagine a Map where getting the value of
> > a key fails.  Hmm...  Maybe we should change keySet()
> > to return a Collection that's not backed by the Map.
>
> No.  keySet() should be backed by the Map.  That's in the Map contract:
> http://java.sun.com/j2se/1.3/docs/api/java/util/Map.html#keySet()

Yeah, it's not an ideal solution.  I'd be happier if LRUMap could perform
gets while still maintaining a reasonable iteration.

> regards,
> michael
>
> >
> > - Morgan
> >
> > --- "Jack, Paul" <pj...@sfaf.org> wrote:
> > > My suggestion would be to have the apache
> > > collections mimic the
> > > java.util collections whereever possible, and so
> > > treat this as a
> > > bug.  Iterators aren't required to raise
> > > ConcurrentModificationExceptions
> > > because in theory, a collection (say, one that's
> > > backed by a database)
> > > might not be able to provide that feature.
> > > Something like SequencedHashMap,
> > > which absolutely CAN provide the feature, probably
> > > SHOULD provide the
> > > feature...
> > >
> > > Furthermore, I seriously doubt that there's any code
> > > out there that relies
> > > on the fact that SequenceHashMap's iterators don't
> > > raise
> > > ConcurrentModificationExceptions,
> > > and even if such code exists, its behavior is
> > > undefined at this point:  It
> > > seems we can fix this in a future release without
> > > worrying about breaking
> > > existing code.
> > >
> > > Just my 2 cents.
> > >
> > > -Paul
> > >
> > > -----Original Message-----
> > > From: Morgan Delagrange [mailto:mdelagra@yahoo.com]
> > > Sent: Wednesday, May 08, 2002 11:09 AM
> > > To: Jakarta Commons Developers
> > > Subject: [Collections] Bug or Feature? -
> > > SequencedHashMap iterators do
> > > not throw ConcurrentModificationExceptions
> > >
> > >
> > > Hi all,
> > >
> > > In tracking down a bug in LRUMap, I added a new unit
> > > test to testMap() to
> > > check if all our Map Iterators throw
> > > ConcurrentModificationExceptions when
> > > the underlying collection is modified.
> > > SequencedHashMap does not.  I know
> > > that it's not absolutely _required_ to handle
> > > modifications that way, but
> > > should we classify this as a bug or an unsupported
> > > feature?  It seems risky
> > > not to throw a ConcurrentModificationException,
> > > because it probably makes
> > > the behaviour of the Iterator indeterminate, and in
> > > the case of LRUMap I
> > > think it's leading to an infinite promotion loop.
> > >
> > > - Morgan
> > >
> > >
> > >
> > > --
> > > To unsubscribe, e-mail:
> > > <ma...@jakarta.apache.org>
> > > For additional commands, e-mail:
> > > <ma...@jakarta.apache.org>
> > >
> > > --
> > > To unsubscribe, e-mail:
> > > <ma...@jakarta.apache.org>
> > > For additional commands, e-mail:
> > > <ma...@jakarta.apache.org>
> > >
> >
> >
> > =====
> > Morgan Delagrange
> > http://jakarta.apache.org/taglibs
> > http://jakarta.apache.org/commons
> > http://axion.tigris.org
> >
> > __________________________________________________
> > Do You Yahoo!?
> > Yahoo! Health - your guide to health and wellness
> > http://health.yahoo.com
> >
> > --
> > To unsubscribe, e-mail:
<ma...@jakarta.apache.org>
> > For additional commands, e-mail:
<ma...@jakarta.apache.org>
> >
>
>
> --
> To unsubscribe, e-mail:
<ma...@jakarta.apache.org>
> For additional commands, e-mail:
<ma...@jakarta.apache.org>


--
To unsubscribe, e-mail:   <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>


RE: [Collections] Bug or Feature? - SequencedHashMap iterators do not throw ConcurrentModificationExceptions

Posted by "Michael A. Smith" <ma...@apache.org>.
On Wed, 8 May 2002, Morgan Delagrange wrote:
> That's my inclination too.  I'm in a real quandry
> about how to handle the LRUMap implementation though. 
> If we make those iterators fail-fast, the following
> operation will produce a
> ConcurrentModificationException:
> 
>   Iterator lruMapKeys = lruMap.keySet().iterator();
> 
>   while (lruMapKeys.hasNext()) {
>     // the second element fails, because
>     // calling get(Object) modifies the underlying
>     // SequencedHashMap
>     System.out.println(lruMap.get(lruMapKeys.next()));
>   }

If you want to iterate over the values to print each of them, use 
values().iterator():

  Iterator lruMapValues = lruMap.values().iterator();

  while(lruMapValues.hasNext()) {
    System.out.println(lruMapValues.next());
  }

Or, if you need to the key and the value, use entrySet().iterator():

  Iterator lruMapEntries = lruMap.entrySet().iterator();

  while(lruMapEntries.hasNext()) {
    Map.Entry entry = (Map.Entry)lruMapEntries.next();
    System.out.println(entry.getValue());
  }

Both of these use the iterator without touching the map, and thus without 
a concurrent modification.

> It's hard to imagine a Map where getting the value of
> a key fails.  Hmm...  Maybe we should change keySet()
> to return a Collection that's not backed by the Map.

No.  keySet() should be backed by the Map.  That's in the Map contract:
http://java.sun.com/j2se/1.3/docs/api/java/util/Map.html#keySet()

regards,
michael

> 
> - Morgan
> 
> --- "Jack, Paul" <pj...@sfaf.org> wrote:
> > My suggestion would be to have the apache
> > collections mimic the
> > java.util collections whereever possible, and so
> > treat this as a
> > bug.  Iterators aren't required to raise
> > ConcurrentModificationExceptions
> > because in theory, a collection (say, one that's
> > backed by a database)
> > might not be able to provide that feature. 
> > Something like SequencedHashMap,
> > which absolutely CAN provide the feature, probably
> > SHOULD provide the 
> > feature...
> > 
> > Furthermore, I seriously doubt that there's any code
> > out there that relies
> > on the fact that SequenceHashMap's iterators don't
> > raise
> > ConcurrentModificationExceptions,
> > and even if such code exists, its behavior is
> > undefined at this point:  It 
> > seems we can fix this in a future release without
> > worrying about breaking 
> > existing code.
> > 
> > Just my 2 cents. 
> > 
> > -Paul
> > 
> > -----Original Message-----
> > From: Morgan Delagrange [mailto:mdelagra@yahoo.com]
> > Sent: Wednesday, May 08, 2002 11:09 AM
> > To: Jakarta Commons Developers
> > Subject: [Collections] Bug or Feature? -
> > SequencedHashMap iterators do
> > not throw ConcurrentModificationExceptions
> > 
> > 
> > Hi all,
> > 
> > In tracking down a bug in LRUMap, I added a new unit
> > test to testMap() to
> > check if all our Map Iterators throw
> > ConcurrentModificationExceptions when
> > the underlying collection is modified. 
> > SequencedHashMap does not.  I know
> > that it's not absolutely _required_ to handle
> > modifications that way, but
> > should we classify this as a bug or an unsupported
> > feature?  It seems risky
> > not to throw a ConcurrentModificationException,
> > because it probably makes
> > the behaviour of the Iterator indeterminate, and in
> > the case of LRUMap I
> > think it's leading to an infinite promotion loop.
> > 
> > - Morgan
> > 
> > 
> > 
> > --
> > To unsubscribe, e-mail:
> > <ma...@jakarta.apache.org>
> > For additional commands, e-mail:
> > <ma...@jakarta.apache.org>
> > 
> > --
> > To unsubscribe, e-mail:  
> > <ma...@jakarta.apache.org>
> > For additional commands, e-mail:
> > <ma...@jakarta.apache.org>
> > 
> 
> 
> =====
> Morgan Delagrange
> http://jakarta.apache.org/taglibs
> http://jakarta.apache.org/commons
> http://axion.tigris.org
> 
> __________________________________________________
> Do You Yahoo!?
> Yahoo! Health - your guide to health and wellness
> http://health.yahoo.com
> 
> --
> To unsubscribe, e-mail:   <ma...@jakarta.apache.org>
> For additional commands, e-mail: <ma...@jakarta.apache.org>
> 


--
To unsubscribe, e-mail:   <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>


RE: [Collections] Bug or Feature? - SequencedHashMap iterators do not throw ConcurrentModificationExceptions

Posted by Morgan Delagrange <md...@yahoo.com>.
That's my inclination too.  I'm in a real quandry
about how to handle the LRUMap implementation though. 
If we make those iterators fail-fast, the following
operation will produce a
ConcurrentModificationException:

  Iterator lruMapKeys = lruMap.keySet().iterator();

  while (lruMapKeys.hasNext()) {
    // the second element fails, because
    // calling get(Object) modifies the underlying
    // SequencedHashMap
    System.out.println(lruMap.get(lruMapKeys.next()));
  }

It's hard to imagine a Map where getting the value of
a key fails.  Hmm...  Maybe we should change keySet()
to return a Collection that's not backed by the Map.

- Morgan

--- "Jack, Paul" <pj...@sfaf.org> wrote:
> My suggestion would be to have the apache
> collections mimic the
> java.util collections whereever possible, and so
> treat this as a
> bug.  Iterators aren't required to raise
> ConcurrentModificationExceptions
> because in theory, a collection (say, one that's
> backed by a database)
> might not be able to provide that feature. 
> Something like SequencedHashMap,
> which absolutely CAN provide the feature, probably
> SHOULD provide the 
> feature...
> 
> Furthermore, I seriously doubt that there's any code
> out there that relies
> on the fact that SequenceHashMap's iterators don't
> raise
> ConcurrentModificationExceptions,
> and even if such code exists, its behavior is
> undefined at this point:  It 
> seems we can fix this in a future release without
> worrying about breaking 
> existing code.
> 
> Just my 2 cents. 
> 
> -Paul
> 
> -----Original Message-----
> From: Morgan Delagrange [mailto:mdelagra@yahoo.com]
> Sent: Wednesday, May 08, 2002 11:09 AM
> To: Jakarta Commons Developers
> Subject: [Collections] Bug or Feature? -
> SequencedHashMap iterators do
> not throw ConcurrentModificationExceptions
> 
> 
> Hi all,
> 
> In tracking down a bug in LRUMap, I added a new unit
> test to testMap() to
> check if all our Map Iterators throw
> ConcurrentModificationExceptions when
> the underlying collection is modified. 
> SequencedHashMap does not.  I know
> that it's not absolutely _required_ to handle
> modifications that way, but
> should we classify this as a bug or an unsupported
> feature?  It seems risky
> not to throw a ConcurrentModificationException,
> because it probably makes
> the behaviour of the Iterator indeterminate, and in
> the case of LRUMap I
> think it's leading to an infinite promotion loop.
> 
> - Morgan
> 
> 
> 
> --
> To unsubscribe, e-mail:
> <ma...@jakarta.apache.org>
> For additional commands, e-mail:
> <ma...@jakarta.apache.org>
> 
> --
> To unsubscribe, e-mail:  
> <ma...@jakarta.apache.org>
> For additional commands, e-mail:
> <ma...@jakarta.apache.org>
> 


=====
Morgan Delagrange
http://jakarta.apache.org/taglibs
http://jakarta.apache.org/commons
http://axion.tigris.org

__________________________________________________
Do You Yahoo!?
Yahoo! Health - your guide to health and wellness
http://health.yahoo.com

--
To unsubscribe, e-mail:   <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>