Memory leak: button's ShortcutRegistration is not removed after detach

[luboA]

Description of the bug / feature

Memory leak caused by ShortcutRegistration persist in the memory after button's detach

Minimal reproducible example

Navigation from View1 -> View2. View2 contains button with click shortcut (independed on key). After navigation back View2 -> View1 ShortcutRegistration inside UI.eventBus is not removed even if View2 was completely detached.

Expected behavior

Button's ShortcutRegistration after View and button detach should be removed too.

Actual behavior

Button's ShortcutRegistration after View and button detach is not removed from UI.eventBus and remains in the memory. This cause memory leak for example when View contains huge data blocks.

Versions:

- Vaadin / Flow version: 14.3.3
- Java version: 1.8

[denis-anisimov]

Hi, unfortunately I can't follow your steps to reproduce since I don't know what's View1, View2, ....etc.
Could you please provide a code snippets which I can copy paste or just make a project which I can use to reproduce ?

You can use this project as a starting point for this : https://github.com/vaadin/skeleton-starter-flow

[Legioth]

Tested with a Vaadin 21.0.2 project from start.vaadin.com

@Route
public class View1 extends VerticalLayout {
    public View1() {
        Object value = ComponentUtil.getData(UI.getCurrent(), "ref");
        if (value instanceof WeakReference<?>) {
            WeakReference<?> ref = (WeakReference<?>) value;
            System.gc();
            try {
                Thread.sleep(100);
            } catch (InterruptedException e) {
                e.printStackTrace();
            }
            System.gc();
            add(new Span("Reference value: " + ref.get()));
        } else {
            add(new Span("No reference set"));
        }

        add(new RouterLink("View 2", View2.class));
    }
}

@Route
public class View2 extends VerticalLayout {
    public View2() {
        Button button = new Button("I have a shortcut", event -> Notification.show("Clicked"));
        button.addClickShortcut(Key.ENTER, KeyModifier.CONTROL);

        add(button);

        ComponentUtil.setData(UI.getCurrent(), "ref", new WeakReference<>(this));

        add(new RouterLink("View 1", View1.class));
    }
}

To reproduce:

1. Open view1 in the browser
2. Observe the the "No reference set" status
3. Click button to go to View 2
4. Click button to go to View 1
5. Observe that the weak reference is not cleared
6. Distrust WeakReference and check the heap dump instead

[luboA]

@Legioth yes your example seems to be correct and it has similar behavior. So it seems to be problem also for vaadin 21. Thank you for your effort.

[denis-anisimov]

Very good, thanks @Legioth

[luboA]

Temporary workaround is to call remove method on ShortcutRegistration instance on detach event, but instance has to be saved in some accessible place (I have saved registration instance under my own class extended from Vaadin's Button).

[denis-anisimov]

The memory leak is isolated by the UI instance. The reference will be destroyed anyway along with the UI instance since it's stored per UI and not per the whole application.
The UI instances are cleaned up all the time (sometimes it needs a timeout).
So even though this memory leak is indeed an issue it's should not be so big problem unless your UI is huge. But in the latter case the huge UI is a problem itself.

[Legioth]

If the user goes 50 times back and forth between View 1 and View 2, then they will end up with 50 instances of View 2 in memory. That UI instance will stay in memory while the user is using the application, which for some applications can be the whole working day.

[luboA]

@Legioth exactly. This was our problem with app and it caused OutOfMemory. We have had maybe bigger problem because we have big lifetime of UI instance and one of our view contains huge dataset.

[denis-anisimov]

The example in the comments doesn't prove anything at least for me since the same steps leads to the same result if there is no button.addClickShortcut(Key.ENTER, KeyModifier.CONTROL);.
But it allows to understand in which way exactly the shortcut is added and ShortcutRegistration is used.

[Legioth]

the same steps leads to the same result if there is no button.addClickShortcut(Key.ENTER, KeyModifier.CONTROL);

Sloppy example from my side. Apologies for that!

The example doesn't work because old view is still referenced by other parts of the framework during the part of the life cycle when the new view is instantiated. It works as intended to show the leak if View1 is changed to check the reference through a button click listener instead.

add(new Button("Check reference", event -> {
    Object value = ComponentUtil.getData(UI.getCurrent(), "ref");
    if (value instanceof WeakReference<?>) {
        WeakReference<?> ref = (WeakReference<?>) value;
        System.gc();
        try {
            Thread.sleep(100);
        } catch (InterruptedException e) {
            e.printStackTrace();
        }
        System.gc();
        add(new Span("Reference value: " + ref.get()));
    } else {
        add(new Span("No reference set"));
    }
}));

[vaadin-bot]

This ticket/PR has been released with platform 22.0.0.alpha6 and is also targeting the upcoming stable 22.0.0 version.

[vaadin-bot]

This ticket/PR has been released with platform 20.0.8.

[luboA]

When it will be released for vaadin 14.7.X?

[vaadin-bot]

This ticket/PR has been released with platform 21.0.3.

[vaadin-bot]

This ticket/PR has been released with platform 14.7.2.