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:
menuItem ---> WeakEventHandler ---weakly---> Lambda
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).
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.
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.
I'd look into the above first before trying other solutions.
--John
On 18/04/2024 17:50, Thiago Milczarek Sayão wrote:
I was investigating,
It probably should be menuItem.setOnAction(new WeakEventHandler<>(e ->
stage.toFront()));
But I bet it's a common mistake. Maybe the setOnAction should mention it?
Em qui., 18 de abr. de 2024 às 11:54, Andy Goryachev
<andy.goryac...@oracle.com> escreveu:
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.
The lambda is essentially this:
menuItem.setOnAction(new H(stage));
class $1 implements EventHandler<ActionEvent> {
private final Stage stage;
public $1(Stage s) {
this.stage = s; // holds the reference and causes the leak
}
public void handle(ActionEvent ev) {
stage.toFront();
}
}
-andy
*From: *openjfx-dev <openjfx-dev-r...@openjdk.org> on behalf of
Thiago Milczarek Sayão <thiago.sa...@gmail.com>
*Date: *Thursday, April 18, 2024 at 03:42
*To: *openjfx-dev <openjfx-dev@openjdk.org>
*Subject: *Possible leak on setOnAction
Hi,
I'm pretty sure setOnAction is holding references.
I have a "Open Windows" menu on my application where it lists the
Stages opened and if you click, it calls stage.toFront():
menuItem.seOnAction(e -> stage.toFront())
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.
To fix it, I call setOnAction(null) when the stage is closed.
I will investigate further and provide an example.
-- Thiago.
