On Sun, 3 May 2026 19:16:37 GMT, Marius Hanl <[email protected]> wrote:
> This PR fixes the (rare) issue that an `ArrayIndexOutOfBoundsException` is > thrown in `FilteredList`. > > Some details of the problem: > - Can only be reproduced when there are multiple changes, as we need an > intermediate state where we have more elements than we will have at the end > after all change events > - The issue is somewhat rare: Normally, we have reserved a big enough array > (`int[] filtered`) to compensate a temporary element increase. With a small > size like 1, we can reproduce it easily by making two change events (In the > reproducer and test, a 'move element to first index' change operation) > > Details for the fix: > - `ensureSize(getSource().size());` is normally correct as our `FilteredList` > will always be <= source list. > But we can have the situation where we have temporarily more elements than > the source list as we are processing all change events (e.g. add 1, remove 1). > - To account for that, we will call `ensureSize` again when we could run > into the situation where we have more elements than we reserved for > - So that we do not over allocate the `filtered` array, I did not use the > proposed change in the ticket but this one instead. We will really only (may) > increase the size when we really need it > - As we reserve more capacity in `filtered` than we may need, this does > not happen often, especially not with more elements > - Since we have a smart change event aggregation algorithm (especially > since the fix in https://github.com/openjdk/jfx/pull/2154 and > https://github.com/openjdk/jfx/pull/2117), we often have more compact change > event where a big increase of elements temporary, which are removed in the > next change are very unlikely (there will be only one change event with the > elements that 'survived' the add and remove). > - Therefore, I do not see any performance concerns here, as > `ensureSize(size + 1);` is very likely a noop, but in cases like this e will > correctly reserve more capacity > > > --------- > - [x] I confirm that I make this contribution in accordance with the [OpenJDK > Interim AI Policy](https://openjdk.org/legal/ai). minor optimization might be possible. modules/javafx.base/src/main/java/javafx/collections/transformation/FilteredList.java line 138: > 136: @Override > 137: protected void sourceChanged(Change<? extends E> c) { > 138: ensureSize(getSource().size()); are you sure this is needed? the tests in the two JBS tickets pass without it, `addRemove()` invokes this method in ~L239 (then calls it again with size+1 in L273). `update()` calls it in ~L286, and `permutate()` does not need it. perhaps `addRemove()` can be further optimized so as not to call `ensureSize()` twice? ------------- PR Review: https://git.openjdk.org/jfx/pull/2163#pullrequestreview-4292244109 PR Review Comment: https://git.openjdk.org/jfx/pull/2163#discussion_r3243290160
