RFR: 8342782: AWTEventMulticaster throws StackOverflowError using AquaButtonUI [v6]
Alexander Zuev
kizune at openjdk.org
Mon Dec 16 19:48:42 UTC 2024
On Sun, 8 Dec 2024 07:52:17 GMT, Jeremy <duke at openjdk.org> wrote:
>> The AWTEventMulticaster is a tree node with 2 children. This PR proposes rebalancing that tree after 500 additions to avoid potential StackOverflowErrors when trying to interact with a large AWTEventMulticaster.
>>
>> In the original headful test:
>> We added 8,000 checkboxes, and when their parent panel was hidden the stack needed to grow to 24,000 lines. It took 8,000 lines to recursively call `java.awt.AWTEventMulticaster.componentHidden`, and then 16,000 additional lines to call two recursive methods to remove a listener:
>>
>> java.desktop/java.awt.AWTEventMulticaster.removeInternal()
>> java.desktop/java.awt.AWTEventMulticaster.remove()
>>
>>
>> With this current PR the max stack size reaches 1,267 instead. (If we rebalanced at EVERY addition, then that same scenario would reach a max stack size of 71.)
>>
>> JDK-8342782 included a headful test case, but I think the main problem it demonstrated can be represented by the headless test case attached to this PR.
>>
>> Depending on how this PR is received I may submit a separate ticket & PR to modify AquaButtonUI so it doesn't always attach an AncestorListener. (That is: if my GUI includes 8,000 checkboxes then I don't need 8,000 AncestorListeners.) But JDK-8342782's test case is currently written in a way that should reproduce across all L&F's, so that can be discussed separately.
>
> Jeremy has updated the pull request incrementally with five additional commits since the last revision:
>
> - 8342782: move comments re implementation details inside method
>
> Also avoid restating constant, instead only refer to it as "threshold"
>
> This is in response to:
> https://github.com/openjdk/jdk/pull/21962#discussion_r1855148909
> - 8342782: adding an empty line representing EOF
>
> This is in response to:
> https://github.com/openjdk/jdk/pull/21962#discussion_r1871348260
> - 8342782: changing "/2" to " / 2"
>
> This is in response to:
> https://github.com/openjdk/jdk/pull/21962#discussion_r1871376542
> - 8342782: change threshold from 500 to 100
>
> This is in response to:
> https://github.com/openjdk/jdk/pull/21962#discussion_r1855398578
> - 8342782: convert value (500) to named constant
>
> This is in response to:
> https://github.com/openjdk/jdk/pull/21962#discussion_r1855397860
Marked as reviewed by kizune (Reviewer).
-------------
PR Review: https://git.openjdk.org/jdk/pull/21962#pullrequestreview-2507190370
More information about the client-libs-dev
mailing list