8195614: FilteredList throws ArrayIndexOutOfBoundsException if created with 1 element by Maran23 · Pull Request #2163 · openjdk/jfx

Maran23 · 2026-05-03T19:16:37Z

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 8237868: java.lang.IndexOutOfBoundsException in FilteredList #2154 and 8208758: ListChangeBuilder doesn't shift indices after idempotent change pairs #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

/issue add JDK-8195750

I confirm that I make this contribution in accordance with the OpenJDK Interim AI Policy.

Progress

Change must not contain extraneous whitespace
Commit message must refer to an issue
Change must be properly reviewed (2 reviews required, with at least 1 Reviewer, 1 Author)

Issues

JDK-8195614: FilteredList throws ArrayIndexOutOfBoundsException if created with 1 element (Bug - P3)
JDK-8195750: FilteredList throws ArrayIndexOutOfBoundsException on ListChangeEvent with multiple change (Bug - P3)

Reviewers

Andy Goryachev (@andy-goryachev-oracle - Reviewer)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jfx.git pull/2163/head:pull/2163
$ git checkout pull/2163

Update a local copy of the PR:
$ git checkout pull/2163
$ git pull https://git.openjdk.org/jfx.git pull/2163/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 2163

View PR using the GUI difftool:
$ git pr show -t 2163

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jfx/pull/2163.diff

Using Webrev

Link to Webrev Comment

…d with 1 element

bridgekeeper · 2026-05-03T19:17:33Z

👋 Welcome back mhanl! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

openjdk · 2026-05-03T19:18:05Z

❗ This change is not yet ready to be integrated.
See the Progress checklist in the description for automated requirements.

openjdk · 2026-05-03T19:18:34Z

@Maran23
Adding additional issue to issue list: 8195750: FilteredList throws ArrayIndexOutOfBoundsException on ListChangeEvent with multiple change.

openjdk · 2026-05-03T19:18:36Z

The total number of required reviews for this PR has been set to 2 based on the presence of this label: rfr. This can be overridden with the /reviewers command.

mlbridge · 2026-05-03T19:22:26Z

Webrevs

00: Full (7008794e)

andy-goryachev-oracle

minor optimization might be possible.

andy-goryachev-oracle · 2026-05-14T17:59:34Z


    @Override
    protected void sourceChanged(Change<? extends E> c) {
+        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?

update() and permutate() do not call it anymore. I relocated them into one single location: Here.
So functionality wise, it did not change.

We could probably remove it, as we will catch any grow on size in addRemove(). But it might hurt performance.
If we have a FilteredList with a size 1 and add 10_000 elements, we will call ensureSize a lot of times, which will lead to a degraded performance.

But since we call it once here, we will immediately grow to the worst case size - if nothing need to be filtered and MAY only grow it in addRemove() when we temporarily have more elements due to processing the changes one by one.

I might be misunderstanding something.

Is ensureSize() L138 needed for permutate() ?

addRemove() calls ensureSize() twice (L138 and L237). can it call it only once?

update() does not change the filtered size, right? why is it calling ensureSize() in L138?

No - but when we are only sorting (permutation) the list, the ensureSize should be a noop, since the size had to be increased at some point in the past to what we currently have and ensured the size

No - because a sub change temporarily adds an element which is later removed on another sub change. We unfortunately can not predict that/count that in beforehand in this ensureSize call. With that said, the second call in addRemove should happen very rarely - probably only with small sizes

It can. When we change an item, lets say A -> B and A is filtered out but B is not, we grow in size by 1 (or the other way around). But same as in the permutation,- the ensureSize should be a noop, since the size had to be increased at some point in the past to what we currently have and ensured the size

So, while I agree that it's likely to be a no-op, my original comment did pointed out a possible optimization (not that it will give us megabytes or free space or 10% less CPU consumption).

So in cases 1) and 2) we could call ensureSize() only when needed. It's up to you whether you want to pursue this further or leave as is.

Yeah I thought about it as well - but this is very hard to balance.
We reserve quite much space here. If we have 1000 elements, we reserve space for 1501 elements. That's huge. And we are in FilteredList - so the normal usecase is probably with quite a few elements filtered out.

But on the other hand growing in size just a few times is much better performance-wise vs. growing in size many more times. As I found out here already: #758

…angeEvent with multiple change NB: this fix is an alternative to openjdk#2163 It has been heavily tested in our application during 6 months with success. it fixes also JDK-8195614

8195614: FilteredList throws ArrayIndexOutOfBoundsException if create…

7008794

…d with 1 element

openjdk Bot added the rfr Ready for review label May 3, 2026

andy-goryachev-oracle reviewed May 14, 2026

View reviewed changes

andy-goryachev-oracle approved these changes May 18, 2026

View reviewed changes

dweil mentioned this pull request May 19, 2026

8195750: FilteredList throws ArrayIndexOutOfBoundsException on ListChangeEvent with multiple change #2172

Closed

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

8195614: FilteredList throws ArrayIndexOutOfBoundsException if created with 1 element#2163

8195614: FilteredList throws ArrayIndexOutOfBoundsException if created with 1 element#2163
Maran23 wants to merge 1 commit into
openjdk:masterfrom
Maran23:8195614-FilteredList-throws-ArrayIndexOutOfBoundsException-if-created-with-1-element

Maran23 commented May 3, 2026 •

edited by openjdk Bot

Loading

Uh oh!

bridgekeeper Bot commented May 3, 2026

Uh oh!

openjdk Bot commented May 3, 2026

Uh oh!

openjdk Bot commented May 3, 2026

Uh oh!

openjdk Bot commented May 3, 2026

Uh oh!

mlbridge Bot commented May 3, 2026

Uh oh!

andy-goryachev-oracle left a comment

Uh oh!

andy-goryachev-oracle May 14, 2026

Uh oh!

Maran23 May 18, 2026

Uh oh!

andy-goryachev-oracle May 18, 2026

Uh oh!

Maran23 May 18, 2026

Uh oh!

andy-goryachev-oracle May 18, 2026

Uh oh!

Maran23 May 18, 2026

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

2 participants

Conversation

Maran23 commented May 3, 2026 • edited by openjdk Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Progress

Issues

Reviewers

Reviewing

Uh oh!

bridgekeeper Bot commented May 3, 2026

Uh oh!

openjdk Bot commented May 3, 2026

Uh oh!

openjdk Bot commented May 3, 2026

Uh oh!

openjdk Bot commented May 3, 2026

Uh oh!

mlbridge Bot commented May 3, 2026

Webrevs

Uh oh!

andy-goryachev-oracle left a comment

Choose a reason for hiding this comment

Uh oh!

andy-goryachev-oracle May 14, 2026

Choose a reason for hiding this comment

Uh oh!

Maran23 May 18, 2026

Choose a reason for hiding this comment

Uh oh!

andy-goryachev-oracle May 18, 2026

Choose a reason for hiding this comment

Uh oh!

Maran23 May 18, 2026

Choose a reason for hiding this comment

Uh oh!

andy-goryachev-oracle May 18, 2026

Choose a reason for hiding this comment

Uh oh!

Maran23 May 18, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

2 participants

Maran23 commented May 3, 2026 •

edited by openjdk Bot

Loading