harmony-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Uwe Schindler (JIRA)" <j...@apache.org>
Subject [jira] Commented: (HARMONY-6681) AbstractCollection.toArray() and AbstractCollection.toArray(T[]) are broken for concurrently modified collections like ConcurrentHashMap.keySet()
Date Tue, 07 Dec 2010 07:58:08 GMT

    [ https://issues.apache.org/jira/browse/HARMONY-6681?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12968619#action_12968619

Uwe Schindler commented on HARMONY-6681:

The fix in the patch attached to HARMONY-6678 is also not fully Java SE conform: In Java 6's
spec of AbstractCollection#toArray the returned array is guaranteed to have the correct size
(when the method taking no array or a smaller array like the common case of passing "new String[0]"
to produce a String[] and not Object[] return value). If the collection gets smaller, the
array may be over-allocated and must be shrinked before returning: "This implementation returns
an array containing all the elements returned by this collection's iterator in the same order,
stored in consecutive elements of the array, starting with index 0. If the number of elements
returned by the iterator is too large to fit into the specified array, then the elements are
returned in a newly allocated array with length equal to the number of elements returned by
the iterator, even if the size of this collection changes during iteration, as might happen
if the collection permits concurrent modification during iteration. The size method is called
only as an optimization hint; the correct result is returned even if the iterator returns
a different number of elements."

The patch in HARMONY-6678 fixes this special issue of ArrayIndexOutOfBounds but is not the
100% correct fix. In Lucene we also see incorrectly additional null elements in the array
when the collection size shrinked during iteration, because the final array size fixup is

Of course the array is never shrinked to a smaller size of the passed in-array, in this case
it has to be filled up with nulls (but only and only if the passed-in array is larger). The
ideal solution for this case would be: If the passed in array has length>0, then start
to fill it by the iterator, once full split to a new one and fix its size at the end. If it
was not reallocated, keep the size and fill with nulls and return original array!

So this is the more general one-for-all issue.

> AbstractCollection.toArray() and AbstractCollection.toArray(T[]) are broken for concurrently
modified collections like ConcurrentHashMap.keySet()
> -------------------------------------------------------------------------------------------------------------------------------------------------
>                 Key: HARMONY-6681
>                 URL: https://issues.apache.org/jira/browse/HARMONY-6681
>             Project: Harmony
>          Issue Type: Bug
>          Components: Classlib
>    Affects Versions: 6.0M4, 5.0M16
>            Reporter: Uwe Schindler
> If you call ConcurrentHashMap.keySet().toArray() [as done in trunks Lucene's RAMDirectory#fileMap]
and another thread modifies the collecton, you get NoSuchElementExceptions or AIOOBEs. The
reason is that AbstractCollection.toArray methods allocate the array using the current size
of the collection and then either iterate on the values (which is oficially supported by ConcurrentHashMap,
its iterator is safe for concurrent modification!) and set them on the target without checking
bounds. On the other hand if there are too few entries in the collection, the array is nulled
at the end. The bad thing is that the toArray() and toArray(T[]) methods behave differently,
so depending on the modification they throw different exceptions (one impl simply drops entries
that dont fit into the array).
> Since Java SE 6, its officially documented, how toArray must behave: [http://download.oracle.com/javase/6/docs/api/java/util/AbstractCollection.html#toArray(T[])];
This is violated by harmony and even the fix in HARMONY-6236 does not respect it, leading
to such bugs.
> Java SE 5 does not document that, but ConcurrentHashMap.keySet(), values() and entrySet()
have own implementations of toArray() (which can be found out by reflection), the root AbstractCollection.toArray()
is not behaving fine there.
> For harmony the fix would be to correct this issue "correctly" by changing the base AbstractCollection
implementation (I would recommend that).
> This is also the root cause for HARMONY-6236 and similar issues, the fix there is invalid,
but I cannot reopen the issue.

This message is automatically generated by JIRA.
You can reply to this email to add a comment to the issue online.

View raw message