RFR: 8301763: Adding children to wrong index leaves inconsistent state in Parent#childrenSet [v2]

Lukasz Kostyra lkostyra at openjdk.org
Wed May 17 16:42:32 UTC 2023


On Wed, 17 May 2023 15:22:57 GMT, Lukasz Kostyra <lkostyra at openjdk.org> wrote:

>> This issue happened because `childSet` member of Parent was modified during `onProposedChange()` call - that call did not recognize negative indexes as invalid, which caused an exception when actually adding the Node to a List.
>> 
>> This seemed like the simplest solution which doesn't rework a lot of code underneath. Exceptions coming from a backing list/collection technically are handled by `VetoableListDecorator`'s try-catch clauses, however `VetoableListDecorator` does not provide an interface to react when such an exception happens - without it we cannot revert `childSet` back to its original state.
>
> Lukasz Kostyra has updated the pull request incrementally with three additional commits since the last revision:
> 
>  - Add index/null checks to ObservableListWrapper and VetoableListDecorator
>  - ParentTest: Expect IndexOutOfBoundsException, add test cases
>  - Parent: Remove index check from onProposedChange

As a small note, the issue is solved with current patches, however I think the best solution would be what I mentioned in the original PR message - an interface in `VetoableListDecorator` which would let us "roll-back" any changes from `onProposedChange` (ex. revert `childSet` to original state) when backing lists throw an exception. This would require a more thorough analysis of what bits use `VetoableListDecorator` and if any roll-backs are necessary there in addition to Parent code.

-------------

PR Comment: https://git.openjdk.org/jfx/pull/1136#issuecomment-1551732200


More information about the openjfx-dev mailing list