RFR: 8342782: AWTEventMulticaster throws StackOverflowError using AquaButtonUI

[Phil Race]

On Thu, 7 Nov 2024 17:49:29 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.

Sorry, took a lot longer than I intended to test it.
Looks good.

src/java.desktop/share/classes/java/awt/AWTEventMulticaster.java line 960:

> 958:      * Occasionally (after approximately 500 invocations) the root
> 959:      * AWTEventMulticaster is rebalanced.
> 960:      *

I am not sure this comment is needed. If you think it is needed then it would need a CSR, and the comment should be tagged @implNote

src/java.desktop/share/classes/java/awt/AWTEventMulticaster.java line 985:

> 983:      * <p>
> 984:      * The first time this method is invoked it will convert a tree that has a degree
> 985:      * of approximately 500 to a tree with a degree of approximately 10.

where does it do that ? I just see it deciding if there's a need to rebalance.

src/java.desktop/share/classes/java/awt/AWTEventMulticaster.java line 991:

> 989:         while (true) {
> 990:             if (++level > 500)
> 991:                 return true;

our coding standards require that you always include the body in { .. }

src/java.desktop/share/classes/java/awt/AWTEventMulticaster.java line 1018:

> 1016:      */
> 1017:     private static EventListener rebalance(EventListener[] array, int index0, int index1) {
> 1018:         if (index0 == index1)

{ .. } again

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

Marked as reviewed by prr (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/21962#pullrequestreview-2422181991
PR Review Comment: https://git.openjdk.org/jdk/pull/21962#discussion_r1833385605
PR Review Comment: https://git.openjdk.org/jdk/pull/21962#discussion_r1833401410
PR Review Comment: https://git.openjdk.org/jdk/pull/21962#discussion_r1833399928
PR Review Comment: https://git.openjdk.org/jdk/pull/21962#discussion_r1833413838