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) ?