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, 30 Nov 2010 20:14:10 GMT

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

Uwe Schindler commented on HARMONY-6681:
----------------------------------------

Exactly, it should simply use size() to initially allocate the array (only as a size hint).
Then it should use the iterator and iterate over the collection. If the iterator contains
more elements than allocated, it has to grow the array like ArrayList (maybe only overallocate
a few items, 2 or so). When the iterator is exhausted, the number of items returned should
be compared with the array size. If its equal (which is always the case when not modified
concurrently), the return the array, else create a new one with exact size and copy.

This implementation is as effective as the original one for the general case, and only needs
to copy for the seldom case of concurrent modification. I can provide a patch, but I have
not signed the special Harmony extra-CLA.

> 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.


Mime
View raw message