<div dir="ltr"><div dir="ltr">I think the leaks (seems to be more than one) happens on:<div><br></div><div>ContextMenuContent.disposeVisualItems<br></div><div><br></div><div>There are visual items holding references.</div><div><br></div><div>They seem to be on MenuItemContainer</div><div><br></div><div>It looks like its held on:</div><div><div style="background-color:rgb(30,31,34);color:rgb(188,190,196)"><pre style="font-family:"JetBrains Mono",monospace;font-size:9.8pt"><span style="color:rgb(199,125,187)">mouseEnteredEventHandler</span></pre><pre style="font-family:"JetBrains Mono",monospace;font-size:9.8pt"><div><pre style="font-family:"JetBrains Mono",monospace;font-size:9.8pt"><span style="color:rgb(199,125,187)">mouseReleasedEventHandler</span></pre><pre style="font-family:"JetBrains Mono",monospace;font-size:9.8pt"><div><pre style="font-family:"JetBrains Mono",monospace;font-size:9.8pt"><span style="color:rgb(199,125,187)">actionEventHandler</span></pre></div></pre></div></pre></div></div></div><div dir="ltr"><br></div><div>And somewhere else I didn't find.</div><div><br></div><div>That's my theory for now.</div><div><br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">Em sáb., 20 de abr. de 2024 às 10:47, Thiago Milczarek Sayão <<a href="mailto:thiago.sayao@gmail.com" target="_blank">thiago.sayao@gmail.com</a>> escreveu:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr">Yeah, I've been doing some investigating.<div><br></div><div>Using the below example, when the "Refresh Menu" button is clicked, it will replace the items making the old one collectable by the GC.</div><div>If the Menu was never used/drop down shown, it will get collected.</div><div>If it was used (just shown), it remains in memory (seen on VisualVM).</div><div><br></div><div>GC Root points to <span style="font-family:"JetBrains Mono",monospace;background-color:rgb(30,31,34);color:rgb(188,190,196)">ContextMenuContent.</span><span style="font-family:"JetBrains Mono",monospace;background-color:rgb(30,31,34);color:rgb(188,190,196)">MenuItemContainer</span></div><div><br></div><div><div style="background-color:rgb(30,31,34);color:rgb(188,190,196)"><pre style="font-family:"JetBrains Mono",monospace">@Override<br><span style="color:rgb(207,142,109)">public void </span>start(Stage primaryStage) {<br>    <span style="color:rgb(207,142,109)">this</span>.primaryStage = primaryStage;<br>    primaryStage.setTitle(<span style="color:rgb(106,171,115)">"Hello World!"</span>);<br>    Button btn = <span style="color:rgb(207,142,109)">new </span>Button(<span style="color:rgb(106,171,115)">"New Stage"</span>);<br><br>    Button btnRefresh = <span style="color:rgb(207,142,109)">new </span>Button(<span style="color:rgb(106,171,115)">"Refresh Menu"</span>);<br><br>    ButtonBar buttonBar = <span style="color:rgb(207,142,109)">new </span>ButtonBar();<br>    buttonBar.getButtons().addAll(menu, btnRefresh);<br><br>    btnRefresh.setOnAction(e -> {<br>        menu.getItems().setAll(getMenuItem());<br>    });<br><br>    Scene scene = <span style="color:rgb(207,142,109)">new </span>Scene(buttonBar);<br>    primaryStage.setScene(scene);<br>    primaryStage.show();<br>    menu.getItems().add(getMenuItem());<br>}<br><br><span style="color:rgb(207,142,109)">private </span>MenuItem getMenuItem() {<br>    MenuItem item = <span style="color:rgb(207,142,109)">new </span>MenuItem();<br>    item.setText(<span style="color:rgb(106,171,115)">"Menu Item"</span>);<br>    item.setOnAction(a -> {<br>        System.out.println(<span style="color:rgb(106,171,115)">"ACTION"</span>);<br>    });<br><br>    <span style="color:rgb(207,142,109)">return </span>item;<br>}</pre></div></div><div><br></div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">Em sáb., 20 de abr. de 2024 às 05:01, John Hendrikx <<a href="mailto:john.hendrikx@gmail.com" target="_blank">john.hendrikx@gmail.com</a>> escreveu:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
  
    
  
  <div>
    <p>I think this code is bug free, and the added fix shouldn't be
      needed -- the old MenuItems should be cleaned up, and so should
      the referenced Stages.</p>
    <p>So I think the bug is elsewhere, maybe not even in your code at
      all. I did see PR's dealing with how MenuItems are linked to their
      platform specific counterparts and how that can be a bit messy. 
      I'm starting to suspect maybe the MenuItems themselves are somehow
      still referenced (perhaps only when they've been displayed once). 
      These should also be GC'd.  They are however much smaller, so as
      long as they don't reference a Stage it is probably not noticeable
      memory wise.</p>
    <p>An inspection with VisualVM should give an answer (or you could
      make a MenuItem very large by attaching some huge objects to it
      that are not referenced elsewhere, via its properties map for
      example).</p>
    <p>--John<br>
    </p>
    On 19/04/2024 14:47, Thiago Milczarek Sayão wrote:<br>
    <blockquote type="cite">
      
      <div dir="ltr">When the window list changes, I'm calling
        item.setOnAction(null) on the "old list" before inserting a new
        one.
        <div>In general it's not a problem because the menu item or
          button is in a "context", like a Stage and everything is freed
          when the stage is closed. Maybe on long lasting stages.<br>
        </div>
        <div><br>
        </div>
        <div>The code goes like this:</div>
        <div><br>
        </div>
        <div>
          <div style="background-color:rgb(30,31,34);color:rgb(188,190,196)">
            <pre style="font-family:"JetBrains Mono",monospace">Window.<span style="color:rgb(86,168,245)">getWindows</span>().<span style="color:rgb(86,168,245)">addListener</span>((ListChangeListener<? <span style="color:rgb(207,142,109)">super </span>Window>) change -> <span style="color:rgb(86,168,245)">updateWindowList</span>());</pre>
          </div>
        </div>
        <div><br>
        </div>
        <div>
          <div style="background-color:rgb(30,31,34);color:rgb(188,190,196)">
            <pre style="font-family:"JetBrains Mono",monospace"><span style="color:rgb(207,142,109)">private void </span><span style="color:rgb(86,168,245)">updateWindowList</span>() {
    Window[] windows = Window.getWindows().toArray(<span style="color:rgb(207,142,109)">new </span>Window[] {});

    List<MenuItem> items = <span style="color:rgb(207,142,109)">new </span>ArrayList<>();
    <span style="color:rgb(207,142,109)">for </span>(Window window : windows) {
        <span style="color:rgb(207,142,109)">if </span>(window <span style="color:rgb(207,142,109)">instanceof </span>Stage stage && stage != primaryStage) {
            MenuItem item = <span style="color:rgb(207,142,109)">new </span>MenuItem();
            item.setText(stage.getTitle());
            item.setOnAction(a -> <span style="color:rgb(199,125,187)">stage</span>.toFront());
            item.setGraphic(<span style="color:rgb(207,142,109)">new </span>FontIcon());
            items.add(item);
        }
    }

    <span style="color:rgb(207,142,109)">for </span>(MenuItem item : btnWindows.getItems()) {
        item.setOnAction(<span style="color:rgb(207,142,109)">null</span>);
    }

    btnWindows.getItems().setAll(items);
}
</pre>
          </div>
        </div>
        <div><br>
        </div>
        <div>Maybe there's a bug, because the old list of items is
          collectable.</div>
        <div><br>
        </div>
        <div><br>
        </div>
      </div>
      <br>
      <div class="gmail_quote">
        <div dir="ltr" class="gmail_attr">Em sex., 19 de abr. de 2024 às
          01:37, John Hendrikx <<a href="mailto:john.hendrikx@gmail.com" target="_blank">john.hendrikx@gmail.com</a>>
          escreveu:<br>
        </div>
        <blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
          <div>
            <p>This probably is a common mistake, however the Weak
              wrapper is also easy to use wrongly.  You can't just wrap
              it like you are doing in your example, because this is how
              the references look:</p>
            <p>     menuItem ---> WeakEventHandler ---weakly--->
              Lambda</p>
            <p>In effect, the Lambda is weakly referenced, and is the
              only reference, so it can be cleaned up immediately (or
              whenever the GC decides to run) and your menu item will
              stop working at a random time in the future.  The
              WeakEventHandler will remain, but only as a stub (and gets
              cleaned up when the listener list gets manipulated again
              at a later stage).</p>
            <p>The normal way to use a Weak wrapper is to put a
              reference to the wrapped part in a private field, which in
              your case would not solve the problem.<br>
            </p>
            <p>I'm assuming however that you are also removing the menu
              item from the Open Windows list. This menu item should be
              cleaned up fully, and so the reference to the Stage should
              also disappear.  I'm wondering why that isn't happening? 
              If the removed menu item remains referenced somehow, then
              it's Action will reference the Stage, which in turns keeps
              the Stage in memory.</p>
            <p>I'd look into the above first before trying other
              solutions.</p>
            <p>--John<br>
            </p>
            <p><br>
            </p>
            <div>On 18/04/2024 17:50, Thiago Milczarek Sayão wrote:<br>
            </div>
            <blockquote type="cite">
              <div dir="ltr">I was investigating,
                <div><br>
                </div>
                <div>It probably should be menuItem.setOnAction(new
                  WeakEventHandler<>(e -> stage.toFront()));</div>
                <div><br>
                </div>
                <div>But I bet it's a common mistake. Maybe the
                  setOnAction should mention it?</div>
                <div><br>
                </div>
                <div><br>
                </div>
              </div>
              <br>
              <div class="gmail_quote">
                <div dir="ltr" class="gmail_attr">Em qui., 18 de abr. de
                  2024 às 11:54, Andy Goryachev <<a href="mailto:andy.goryachev@oracle.com" target="_blank">andy.goryachev@oracle.com</a>>
                  escreveu:<br>
                </div>
                <blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
                  <div>
                    <div lang="EN-US">
                      <div>
                        <p class="MsoNormal"><span>You are correct - the
                            lambda strongly references `stage` and since
                            it is in turn is strongly referenced from
                            the menu item it creates a leak.</span></p>
                        <p class="MsoNormal"><span> </span></p>
                        <p class="MsoNormal"><span>The lambda is
                            essentially this:</span></p>
                        <p class="MsoNormal"><span> </span></p>
                        <p class="MsoNormal"><span>menuItem.setOnAction(new
                            H(stage));</span></p>
                        <p class="MsoNormal"><span>class $1 implements
                            EventHandler<ActionEvent> {</span></p>
                        <p class="MsoNormal"><span>  private final Stage
                            stage;</span></p>
                        <p class="MsoNormal"><span>  public $1(Stage s)
                            {</span></p>
                        <p class="MsoNormal"><span>    this.stage = s;
                            // holds the reference and causes the leak</span></p>
                        <p class="MsoNormal"><span>  }</span></p>
                        <p class="MsoNormal"><span>  public void
                            handle(ActionEvent ev) {</span></p>
                        <p class="MsoNormal"><span>    stage.toFront();</span></p>
                        <p class="MsoNormal"><span>  }</span></p>
                        <p class="MsoNormal"><span>}</span></p>
                        <p class="MsoNormal"><span> </span></p>
                        <p class="MsoNormal"><span>-andy</span></p>
                        <p class="MsoNormal"><span> </span></p>
                        <div id="m_4518818131945713436m_7245642436684913092m_-3046556027074829362m_6611137440671845182m_-1465540377573045932mail-editor-reference-message-container">
                          <div>
                            <div style="border-right:none;border-bottom:none;border-left:none;border-top:1pt solid rgb(181,196,223);padding:3pt 0in 0in">
                              <p class="MsoNormal" style="margin-bottom:12pt"><b><span style="font-size:12pt;color:black">From:
                                  </span></b><span style="font-size:12pt;color:black">openjfx-dev
                                  <<a href="mailto:openjfx-dev-retn@openjdk.org" target="_blank">openjfx-dev-retn@openjdk.org</a>>
                                  on behalf of Thiago Milczarek Sayão
                                  <<a href="mailto:thiago.sayao@gmail.com" target="_blank">thiago.sayao@gmail.com</a>><br>
                                  <b>Date: </b>Thursday, April 18, 2024
                                  at 03:42<br>
                                  <b>To: </b>openjfx-dev <<a href="mailto:openjfx-dev@openjdk.org" target="_blank">openjfx-dev@openjdk.org</a>><br>
                                  <b>Subject: </b>Possible leak on
                                  setOnAction</span></p>
                            </div>
                            <div>
                              <p class="MsoNormal"><span style="font-size:12pt">Hi,</span></p>
                              <div>
                                <p class="MsoNormal"><span style="font-size:12pt"> </span></p>
                              </div>
                              <div>
                                <p class="MsoNormal"><span style="font-size:12pt">I'm pretty
                                    sure setOnAction is holding
                                    references. </span></p>
                              </div>
                              <div>
                                <p class="MsoNormal"><span style="font-size:12pt"> </span></p>
                              </div>
                              <div>
                                <p class="MsoNormal"><span style="font-size:12pt">I have a
                                    "Open Windows" menu on my
                                    application where it lists the
                                    Stages opened and if you click, it
                                    calls stage.toFront():</span></p>
                              </div>
                              <div>
                                <p class="MsoNormal"><span style="font-size:12pt"> </span></p>
                              </div>
                              <div>
                                <p class="MsoNormal"><span style="font-size:12pt">menuItem.seOnAction(e
                                    -> stage.toFront())</span></p>
                              </div>
                              <div>
                                <p class="MsoNormal"><span style="font-size:12pt"> </span></p>
                              </div>
                              <div>
                                <p class="MsoNormal"><span style="font-size:12pt">I had many
                                    crash reports, all OOM. I got the
                                    hprof files and analyzed them -
                                    turns out this was holding
                                    references to all closed stages.</span></p>
                              </div>
                              <div>
                                <p class="MsoNormal"><span style="font-size:12pt"> </span></p>
                              </div>
                              <div>
                                <p class="MsoNormal"><span style="font-size:12pt">To fix it, I
                                    call setOnAction(null) when the
                                    stage is closed.</span></p>
                              </div>
                              <div>
                                <p class="MsoNormal"><span style="font-size:12pt"> </span></p>
                              </div>
                              <div>
                                <p class="MsoNormal"><span style="font-size:12pt">I will
                                    investigate further and provide an
                                    example.</span></p>
                              </div>
                              <div>
                                <p class="MsoNormal"><span style="font-size:12pt"> </span></p>
                              </div>
                              <div>
                                <p class="MsoNormal"><span style="font-size:12pt">-- Thiago.</span></p>
                              </div>
                            </div>
                          </div>
                        </div>
                      </div>
                    </div>
                  </div>
                </blockquote>
              </div>
            </blockquote>
          </div>
        </blockquote>
      </div>
    </blockquote>
  </div>

</blockquote></div>
</blockquote></div>
</div>