Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Detach listeners not called when closing tab after resynchronization #19023

Closed
mperktold opened this issue Mar 23, 2024 · 3 comments · Fixed by #19068
Closed

Detach listeners not called when closing tab after resynchronization #19023

mperktold opened this issue Mar 23, 2024 · 3 comments · Fixed by #19068

Comments

@mperktold
Copy link

Description of the bug

In recent versions, Vaadin uses the beacon API to inform the server when a tab is closed, so the server can immediately call all detach listeners to clean up resources.

I noticed that the listeners are not called when a resynchronization happened before.

Expected behavior

When the tab is closed, and a beacon request is sent, detach listeners should fire, regardless of whether resynchronization has happened before in this tab or not.

Minimal reproducible example

Consider this example:

public class MainView extends Composite<VerticalLayout> {

    public MainView() {
        var resync = new Button("Resync", e -> getUI()
            .ifPresent(ui -> ui.getInternals().incrementServerId())
        );
        resync.addDetachListener(e -> logDetach(e, "Resync button"));
        var remove = new Button("Remove", e -> resync.getElement().removeFromParent());
        getContent().add(resync, remove);
        UI.getCurrent().addDetachListener(e -> logDetach(e, "UI"));
    }

    @Override
    protected void onDetach(DetachEvent detachEvent) {
        super.onDetach(detachEvent);
        logDetach(detachEvent, "MainView");
    }

    private static void logDetach(DetachEvent event, String element) {
        System.err.println(element + " detached, UI.isClosing = " + event.getUI().isClosing());
    }
}

It shows a simple page with two buttons. The first button triggers resynchronization, the second one removes the first.
There are several detach listeners that log something to System.err so we can see whether they have been called.

To reproduce:

  1. Click on "Resync", wait until resynchronization is completed.
  2. Close the tab or refresh the page.

During resynchronization, all listeners will log that they have been called while the UI was not closing (as discussed in #18443).
Then, when closing the tab, all listeners should be called agein, this time with UI.isClosing = false, but they aren't.

Interestingly, this issue really seems to be specific to closing the tab.
When you click on "Remove", the detach listener of the "Resync" button is always triggered, regardless if resynchronization has happened before or not.

Versions

  • Vaadin / Flow version: 24.3.7
  • Java version: 21.0.2
  • OS version: Windows 11
@mperktold
Copy link
Author

mperktold commented Mar 23, 2024

I tried to debug this myself and I found out that when closing the tab after resynchronization, when StateNode.onDetach is called, hasBeenAttached is false for all nodes in the tree. This is probably because during resynchronization, the flag is explicitly set to false:

protected void prepareForResync() {
visitNodeTreeBottomUp(StateNode::fireDetachListeners);
visitNodeTree(stateNode -> {
getOwner().markAsDirty(stateNode);
stateNode.wasAttached = false;
stateNode.isInitialChanges = true;
stateNode.hasBeenAttached = false;
stateNode.hasBeenDetached = false;
});
visitNodeTreeBottomUp(sn -> sn.fireAttachListeners(true));
}

It would be set to true in StateNode.onAttach, but neither onAttach nor onDetach are called during resynchronization, only listeners and Component.onDetach hooks, which is expected I guess. Probably prepareForResync should set the flag to true at the end, or not set it to false at all, or something to that effect.

@vaadin-bot
Copy link
Collaborator

This ticket/PR has been released with Vaadin 24.4.0.alpha19 and is also targeting the upcoming stable 24.4.0 version.

@vaadin-bot
Copy link
Collaborator

This ticket/PR has been released with Vaadin 24.3.10.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment