On 2018-05-14 22:29, Ivan Gerasimov wrote:
Thank you Claes!
The mutator methods normally first update the modCount and then change
the size of ArrayList.
Then, in other methods the modCount is copied to a local variable
first, and after that the size is copied.
This is not true for equalsRange(List<?> other, int from, int to) when
it is called from ArrayList.equals: the size is first copied to the
argument and then the modCount is checked inside of equalsRange(). If
the size and modCount are changed in between, then equals may produce
a wrong results.
It seems to be more accurate to store this.modCount prior calling to
equalsRange((List<?>) o, 0, size); and do
checkForComodification(expectedModCount); after it is done.
Checking for modCount inside equalsRange() can probably be safely
removed.
There's another call to equalsRange() from SubList.equals(). In this
case checkForComodification(); is already called after calling to
equalsRange(), so everything seems to be fine here.
I was actually toying with and testing a change to this effect anyway,
since it's a nice cleanup and might help the JIT somewhat:
http://cr.openjdk.java.net/~redestad/8196340/open.02/
A note on correctness: none of the code in ArrayList that utilize
modCount is "correct" or accurate in a concurrency or thread-safety
context where ordering of loads and stores is critical for correctness:
lacking volatile or other means to ensure correct ordering of operations
(or atomicity of modCount increments) the runtime is given large amounts
of freedoms to do as it please to optimize things. This is intentional
as ArrayList isn't supposed to be neither thread-safe nor concurrent on
its own. The main utility is instead about detecting and avoid coding
errors, mainly bugs like adding to or removing items from a list you're
simultaneously iterating over. So what I'm saying is that the ordering
of operations within a method might not be all that important as long as
they are sound within the scope of the method execution. Still, making
it a bit more like correct concurrent code might improve the best effort
behavior in some circumstances, and costs us nothing.
Thanks - and sorry for dragging you along for a bit longer...
/Claes
