UI added to session before fully initialized

[WoozyG]

Description

``It is possible to hit an IllegalStateException when performing an action on session UI instances because a UI is in the list but hasn't finished initialization.

"Routing is not in use or not yet initialized. If you are not using embedded UI, try postponing the call to an onAttach method or to an AfterNavigationEvent listener."

The problem is, if you have multiple tabs/windows in your session, and a somewhat long-running (maybe just a couple seconds) UIInitListener, then you can take an action that opens a new tab or window with an application URL, which creates a new UI instance, and then in the original window perform some action that triggers a loop through session UIs, perhaps to message each one, poll state, etc., and get this error.

The error comes from BootstrapHandler.createAndInitUI(...). It has this code at the end:

        // Set thread local here so it is available in init
        UI.setCurrent(ui);
        ui.doInit(request, session.getNextUIid(), context.getAppId());
        session.addUI(ui);

        // After init and adding UI to session fire init listeners.
        session.getService().fireUIInitListeners(ui);

        initializeUIWithRouter(context, ui);

As you can see, the error comes because the router hasn't been initialized yet when the UI is added to the session UI Collection. That collection can be iterated on immediately by another request. The time window is controlled by the duration of the custom listeners.

It could be as simple as not adding the UI to the session until after the router is done, or it may need some sort of two-phase update, if the UI needs to be found in the session in the contract of init listeners or the implementation of router init.

Expected outcome

I expect VaadinSession.getCurrent().getUIs().forEach(ui->ui.access(()->UI::getCurrentView)); to not throw IllegalStateException.

Minimal reproducible example

See the steps to reproduce

Steps to reproduce

The problem is, if you have multiple tabs/windows in your session, and a somewhat long-running (maybe just a couple seconds) UIInitListener, then you can take an action that opens a new tab or window with an application URL, which creates a new UI instance, and then in the original window perform some action that triggers a loop through session UIs, perhaps to message each one, poll state, etc., and get this error.

So create an app with a view with a link to itself. Add a UIInitListener' that just sleeps 5 or 10 seconds. Add a button that tries to enumerate the UIs and call getCurrentView: VaadinSession.getCurrent().getUIs().forEach(ui->ui.access(()->UI::getCurrentView));`

Run the app. control-click the link to open it in a new tab, then click the button to trigger the enumeration.

Environment

Vaadin version(s): 24.7.4 OS: N/A

Browsers

Issue is not browser related

[web-padawan]

This sounds like a Flow issue not specific to a certain component, so I'll transfer it to the Flow repository.

[mshabarov]

UIInitListener is called before router initialisation because of navigation events creation and ordering. If we change the order of the initializeUIWithRouter and fireUIInitListeners this would likely lead to broken navigation and wrong navigation event firing.

The problem is that you are trying to use UI init listener to get routing data, whereas the routing is not yet ready/completed. This approach is unreliable because this may work when a navigation completes and may throw if not.

Would it be an option for you to use Component::onAttach for the view to put your logic there or navigation listeners (BeforeEnterEvent) ?

[WoozyG]

We want to poll/update all UIs (or all relevant UIs based on some criteria) when certain changes happen to backing session or user related configurations.

The issue is that while we can get the list of UIs from the session, we have no idea if they are fully initialized yet or not, and no way to tell.

We assumed UIs must be fully initialized by the time the access() call is run, since UI init and access should both require a lock on the VaadinSession.

In taking another look at our code (I finally have a 2nd developer! Yay!), I see however we are doing this, as noted in the issue:

'VaadinSession.getCurrent().getUIs().forEach(ui->ui.access(()->UI::getCurrentView));'

However, I think perhaps ui.access() should be first, wrapping the entire forEach(). That ultimately shouldn't matter, though, should it?

UIInitListener is called before router initialisation because of navigation events creation and ordering. If we change the order of the initializeUIWithRouter and fireUIInitListeners this would likely lead to broken navigation and wrong navigation event firing.

The problem is that you are trying to use UI init listener to get routing data, whereas the routing is not yet ready/completed. This approach is unreliable because this may work when a navigation completes and may throw if not.

Would it be an option for you to use Component::onAttach for the view to put your logic there or navigation listeners (BeforeEnterEvent) ?

[tepi]

Did some testing on this and so far looks that there's really no contract on the order of in which the events happen. While it looks like your suggestion on moving session.addUI(ui); after everything else does not seem to break anything (see https://github.com/vaadin/flow/pull/22351), this is still quite a big breaking change which I don't think we want to do right now.

You could still explore some options:

• Move your code from UIInitListener to onAttach
• Add exception handling and handle the not yet initialized UI later

Also, while not the nicest option, the change is very simple so you could build a custom build of flow-server for your own use.

[Artur-]

Isn't the problem here that you are accessing the VaadinSession without locking it? If you do

VaadinSession.getCurrent().getUIs();

without acquiring the session lock, you can get half initialized UIs as you note. You can maybe get half destroyed also, or other issues.

Does e.g.

var session = VaadinSession.getCurrent();
session.accessSynchronously(() => {
  session.getUIs()....
});

resolve this?

[WoozyG]

Isn't the problem here that you are accessing the VaadinSession without locking it?

We were iterating on UIs inside an event handler for a component attached to a UI in the session. That would already have the session lock. I think the UI initialization is not happening under a session lock, in that case.