On Wed, 15 Jan 2025 00:32:37 GMT, Michael Strauß <mstra...@openjdk.org> wrote:
> Passing a `VetoableListDecorator.subList()` to any of its bulk operations
> (`addAll`, `setAll`, `removeAll`, `retainAll`) throws
> `ConcurrentModificationException`. The reason is that the
> `VetoableListDecorator.modCount` field is incremented before the underlying
> list's bulk operation is invoked, which causes a mismatch when the sublist is
> interrogated by the bulk operation.
>
> However, simply updating the `modCount` field _after_ the underlying list was
> modified also doesn't work, as in this case listeners can't see the correct
> value for `modCount` in their callback. The fix is to make a defensive copy
> of the sublist before invoking the underlying list's bulk operation.
The scenario I was referring to is impossible, I think.
One possible alternative is to create the defensive copy each time, this will
save one extra pointer every time an iterator or a sublist gets created (these
objects might be long lived). The code in this PR creates a copy in many
(most?) cases anyway, and in my opinion, the memory is more precious resource
that CPU cycles (i.e. using extra memory costs many more CPU cycles in garbage
collection etc.), so please consider that.
+ some minor suggestions
modules/javafx.base/src/test/java/test/javafx/collections/VetoableObservableListTest.java
line 212:
> 210: list.addAll(list.subList(0, 2));
> 211: assertSingleCall(new String[] {"foo", "bar"}, new int[] {4, 4});
> 212: }
suggestion: also check that the list contains the newly added elements?
(here and in added tests that involve subList?)
-------------
PR Review: https://git.openjdk.org/jfx/pull/1679#pullrequestreview-2556899713
PR Review Comment: https://git.openjdk.org/jfx/pull/1679#discussion_r1918996891
