Hello Tagir,
On 2019-06-05 15:49, Tagir Valeev wrote:
Hello, Claes!
Yes, it's true that CME will not be thrown anymore. Depending on the
concurrent modification kind one might face various ranges of incorrect
behavior, including the AIOOBE, NPE or incorrect array returned. As
Collection spec says [1]:
> In the absence of a stronger guarantee by the implementation,
undefined behavior may result from the invocation of any method on a
collection that is being mutated by another thread
In particular it's never mentioned in HashSet, HashMap or Collection
spec that toArray implements a fail-fast behavior: this is said only
about the iterator() method.
that's not entirely true, since the @implSpec for toArray on
AbstractCollection states it is equivalent to iterating over the
collection[1].
So since the implementations you're changing inherit their current
implementation from AbstractCollection and the iterators of HashMap is
specified to be fail-fast, then that behavior can be argued to be
specified also for the affected toArray methods.
I'm not fundamentally objecting to the behavior change, but I do think
it needs careful review and a CSR (or at least plenty of reviewers
agreeing that one isn't needed).
Also note that ArrayList and LinkedList
toArray implementation also never throws CME. LinkedHashSet.toArray in
my patch is pretty similar to LinkedList.toArray. So I feel that such
change is perfectly acceptable: toArray method has no moral obligation
to fail fast, and there's no reason why it should be more "robust" for
HashSet than for LinkedList. or ArrayList.
Finally I should note that there are even more unfortunate examples of
behavior during the concurrent modification. In particular, querying a
TreeMap element during the concurrent update may cause an infinite loop
which is usually much more unpleasant than random exception.
Using worse flaws elsewhere seems like a poor excuse to make. Shouldn't
it rather be turned into an argument in favor of fixing TreeMap...?
/Claes
[1]
https://docs.oracle.com/en/java/javase/12/docs/api/java.base/java/util/AbstractCollection.html#toArray()
