Do not force callers of the LitRenderer constructor to hold a lock on the VaadinSession

[mperktold]

Describe your motivation

In v24, the LitRenderer constructor updates a property "__litRendererCount" of the UI. That's only possible while holding the lock on the VaadinSession.

This is unfortunate, and very atypical for a constructor. In our application, we are doing a lot of work in background threads, including creating components. They are then added later via UI.access and @Push.

Describe the solution you'd like

Maybe you could consider changing LitRenderer such that it acquires the lock itself, such that the callers do not have to know about this implementation detail?

Describe alternatives you've considered

Currently, we would need to check all places were renderers are created, either directly or indirectly, and either look whether these places can be called from a context were the lock on the VaadinSession is not hold, or simply acquire the lock everywhere.

Additional context

No response

[Artur-]

There is no way even to know which UI the LitRenderer will be used in either before it is attached to one, so UI.getCurrent() should not be used

[mperktold]

Good point!

Maybe this code should actually be deferred, like in an AttachListener of the container in which it is first rendered, or runWhenAttached, or something similar.

[Artur-]

But if you are in a background thread, why is UI.getCurrent() defined?

[mperktold]

Because we set it before running the background task, similar to how UI.access sets the receiver UI as the current one while running the provided command, like so:

public static Runnable withCurrentInstances(UI ui, Runnable task) {
    return () -> {
        var oldInstances = CurrentInstances.setCurrent(ui);
        try {
            task.run();
        }
        finally {
            CurrentInstances.restoreInstances(oldInstances);
        }
    };
}

executor.execute(withCurrentInstances(UI.getCurrent(), task));

[Artur-]

You probably should not do that, as many places assume that the framework is the one handling the current UI instance and that it is an indication that a current UI is locked and available

[mperktold]

But I have to do that for several things, i.e. opening dialogs from background threads.

Also, the JavaDoc of UI.setCurrent explicitly allows to use that method for background threads:

https://github.com/vaadin/flow/blob/4072f6d108d8fdfe5d5bc7d65cf7f8b4c7a0d171/flow-server/src/main/java/com/vaadin/flow/component/UI.java#L297-L299

[mperktold]

It's also worth pointing out that this is a regression/breaking change: it worked in Vaadin 23, and stopped working in Vaadin 24.

I understand that breaking changes can happen in major version jumps. Still, it's important to know that this is such a case.