Re: RFR: 8283551: ControlAcceleratorSupport menu items listener causes memory leak [v2]

2023-06-28 Thread Ambarish Rapte
On Fri, 24 Feb 2023 10:04:48 GMT, Dean Wookey wrote: >> Each time a menu would change scenes, a new set of ListChangeListeners would >> be added to the items in the menu. The bigger problem however is that these >> list change listeners have a strong reference to the scene which is >>

Re: RFR: 8283551: ControlAcceleratorSupport menu items listener causes memory leak [v2]

2023-06-27 Thread Kevin Rushforth
On Fri, 24 Feb 2023 10:04:48 GMT, Dean Wookey wrote: >> Each time a menu would change scenes, a new set of ListChangeListeners would >> be added to the items in the menu. The bigger problem however is that these >> list change listeners have a strong reference to the scene which is >>

Re: RFR: 8283551: ControlAcceleratorSupport menu items listener causes memory leak [v2]

2023-06-27 Thread Andy Goryachev
On Fri, 24 Feb 2023 10:04:48 GMT, Dean Wookey wrote: >> Each time a menu would change scenes, a new set of ListChangeListeners would >> be added to the items in the menu. The bigger problem however is that these >> list change listeners have a strong reference to the scene which is >>

Re: RFR: 8283551: ControlAcceleratorSupport menu items listener causes memory leak [v2]

2023-06-21 Thread Michael Strauß
On Fri, 24 Feb 2023 10:04:48 GMT, Dean Wookey wrote: >> Each time a menu would change scenes, a new set of ListChangeListeners would >> be added to the items in the menu. The bigger problem however is that these >> list change listeners have a strong reference to the scene which is >>

Re: RFR: 8283551: ControlAcceleratorSupport menu items listener causes memory leak [v2]

2023-06-15 Thread Kevin Rushforth
On Fri, 24 Feb 2023 10:04:48 GMT, Dean Wookey wrote: >> Each time a menu would change scenes, a new set of ListChangeListeners would >> be added to the items in the menu. The bigger problem however is that these >> list change listeners have a strong reference to the scene which is >>

Re: RFR: 8283551: ControlAcceleratorSupport menu items listener causes memory leak [v2]

2023-06-15 Thread Dean Wookey
On Wed, 1 Mar 2023 13:36:46 GMT, Kevin Rushforth wrote: >> Dean Wookey has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Added more comments and fixed IdentityWrapper hashcode. > > @arapte can you be the second reviewer? @kevinrushforth

Re: RFR: 8283551: ControlAcceleratorSupport menu items listener causes memory leak [v2]

2023-05-02 Thread Dean Wookey
On Fri, 24 Feb 2023 10:04:48 GMT, Dean Wookey wrote: >> Each time a menu would change scenes, a new set of ListChangeListeners would >> be added to the items in the menu. The bigger problem however is that these >> list change listeners have a strong reference to the scene which is >>

Re: RFR: 8283551: ControlAcceleratorSupport menu items listener causes memory leak [v2]

2023-04-04 Thread Dean Wookey
On Fri, 24 Feb 2023 10:04:48 GMT, Dean Wookey wrote: >> Each time a menu would change scenes, a new set of ListChangeListeners would >> be added to the items in the menu. The bigger problem however is that these >> list change listeners have a strong reference to the scene which is >>

Re: RFR: 8283551: ControlAcceleratorSupport menu items listener causes memory leak [v2]

2023-03-01 Thread Kevin Rushforth
On Fri, 24 Feb 2023 10:04:48 GMT, Dean Wookey wrote: >> Each time a menu would change scenes, a new set of ListChangeListeners would >> be added to the items in the menu. The bigger problem however is that these >> list change listeners have a strong reference to the scene which is >>

Re: RFR: 8283551: ControlAcceleratorSupport menu items listener causes memory leak [v2]

2023-03-01 Thread Kevin Rushforth
On Fri, 24 Feb 2023 19:02:25 GMT, Andy Goryachev wrote: >> In general should we use new syntax like that instanceof syntax? JavaFX >> requires Java 17 now, but if changes were to be backported then using the >> new syntax makes it a bit more difficult. > > that will be "SEP" (someone else's

Re: RFR: 8283551: ControlAcceleratorSupport menu items listener causes memory leak [v2]

2023-03-01 Thread Kevin Rushforth
On Fri, 24 Feb 2023 19:09:03 GMT, Andy Goryachev wrote: >> I've been working on changes for a possible follow-up PR which address more >> bugs. For adding accelerators, there are 3 public methods, for removing >> there are currently 4. >> >> I've looked at it, and it can/should be made to

Re: RFR: 8283551: ControlAcceleratorSupport menu items listener causes memory leak [v2]

2023-02-24 Thread Andy Goryachev
On Thu, 23 Feb 2023 09:52:08 GMT, Dean Wookey wrote: >> I'm not in favor of using `Private` in a method name. That is clear from the >> method signature and overloading methods is a valid choice In my opinion, >> this is fine as is. >> But we could also think about naming it:

Re: RFR: 8283551: ControlAcceleratorSupport menu items listener causes memory leak [v2]

2023-02-24 Thread Andy Goryachev
On Fri, 24 Feb 2023 10:04:48 GMT, Dean Wookey wrote: >> Each time a menu would change scenes, a new set of ListChangeListeners would >> be added to the items in the menu. The bigger problem however is that these >> list change listeners have a strong reference to the scene which is >>

Re: RFR: 8283551: ControlAcceleratorSupport menu items listener causes memory leak [v2]

2023-02-24 Thread Andy Goryachev
On Fri, 24 Feb 2023 09:49:33 GMT, Dean Wookey wrote: >> modules/javafx.controls/src/main/java/com/sun/javafx/scene/control/ControlAcceleratorSupport.java >> line 366: >> >>> 364: public boolean equals(Object o) { >>> 365: if (this == o) return true; >>> 366: if

Re: RFR: 8283551: ControlAcceleratorSupport menu items listener causes memory leak [v2]

2023-02-24 Thread Dean Wookey
On Fri, 24 Feb 2023 10:04:48 GMT, Dean Wookey wrote: >> Each time a menu would change scenes, a new set of ListChangeListeners would >> be added to the items in the menu. The bigger problem however is that these >> list change listeners have a strong reference to the scene which is >>

Re: RFR: 8283551: ControlAcceleratorSupport menu items listener causes memory leak [v2]

2023-02-24 Thread Dean Wookey
> Each time a menu would change scenes, a new set of ListChangeListeners would > be added to the items in the menu. The bigger problem however is that these > list change listeners have a strong reference to the scene which is > potentially a much bigger leak. > > The first commit was more

Re: RFR: 8283551: ControlAcceleratorSupport menu items listener causes memory leak

2023-02-24 Thread Dean Wookey
On Wed, 22 Feb 2023 19:31:29 GMT, Andy Goryachev wrote: >> Each time a menu would change scenes, a new set of ListChangeListeners would >> be added to the items in the menu. The bigger problem however is that these >> list change listeners have a strong reference to the scene which is >>

Re: RFR: 8283551: ControlAcceleratorSupport menu items listener causes memory leak

2023-02-24 Thread Dean Wookey
On Wed, 22 Feb 2023 19:21:41 GMT, Andy Goryachev wrote: >> Each time a menu would change scenes, a new set of ListChangeListeners would >> be added to the items in the menu. The bigger problem however is that these >> list change listeners have a strong reference to the scene which is >>

Re: RFR: 8283551: ControlAcceleratorSupport menu items listener causes memory leak

2023-02-23 Thread Dean Wookey
On Thu, 23 Feb 2023 09:26:29 GMT, Marius Hanl wrote: >> modules/javafx.controls/src/main/java/com/sun/javafx/scene/control/ControlAcceleratorSupport.java >> line 285: >> >>> 283: } >>> 284: >>> 285: private static void removeAcceleratorsFromScene(List>> MenuItem> items, Scene scene) {

Re: RFR: 8283551: ControlAcceleratorSupport menu items listener causes memory leak

2023-02-23 Thread Marius Hanl
On Wed, 22 Feb 2023 19:24:16 GMT, Andy Goryachev wrote: >> Each time a menu would change scenes, a new set of ListChangeListeners would >> be added to the items in the menu. The bigger problem however is that these >> list change listeners have a strong reference to the scene which is >>

Re: RFR: 8283551: ControlAcceleratorSupport menu items listener causes memory leak

2023-02-22 Thread Andy Goryachev
On Thu, 16 Feb 2023 15:15:11 GMT, Dean Wookey wrote: > Each time a menu would change scenes, a new set of ListChangeListeners would > be added to the items in the menu. The bigger problem however is that these > list change listeners have a strong reference to the scene which is > potentially

Re: RFR: 8283551: ControlAcceleratorSupport menu items listener causes memory leak

2023-02-22 Thread Kevin Rushforth
On Thu, 16 Feb 2023 15:15:11 GMT, Dean Wookey wrote: > Each time a menu would change scenes, a new set of ListChangeListeners would > be added to the items in the menu. The bigger problem however is that these > list change listeners have a strong reference to the scene which is > potentially

RFR: 8283551: ControlAcceleratorSupport menu items listener causes memory leak

2023-02-16 Thread Dean Wookey
Each time a menu would change scenes, a new set of ListChangeListeners would be added to the items in the menu. The bigger problem however is that these list change listeners have a strong reference to the scene which is potentially a much bigger leak. The first commit was more