Locking in AbstractCommunicationManager

I just got aware of the fact, that handleUidlRequest in AbstractCommunicationManager uses the session for locking.

I came across this problem because we have in our application some long-running UIDL requests. As expected, such a request blocks the UI/tab from which the request was made (i.e., the progress indicator is shown and the user cannot click anything else). What I haven’t expected is that if the user opens a second tab, this call is not processed until the UIDL request for the first UI instance has been finished (in case the newly created UI instance is added to the same session as the first one).

Is this a bug or is this intentionally designed like this? And if so, why? Wouldn’t it be enough to lock on the UI or is a UIDL request not always for a specific UI?

In case it isn’t clear yet what I mean:

  • Create a UI class with the following init-method
    @Override protected void init(VaadinRequest request) { Button button = new Button("click"); button.addClickListener(new ClickListener() { @Override public void buttonClick(ClickEvent event) { try { Thread.sleep(30000); } catch (InterruptedException e) { e.printStackTrace(); } } }); setContent (button); }
  • Open the application in a browser tab
  • Click the button, this call will block for 30 seconds
  • During this time, open the application in a second browser tab
  • The request is not handled until the 30 seconds are over, i.e., the button is not visible until the 30 seconds are over
  • If during the 30 seconds, the application is opened in a different browser, everything works as expected (because then the UI is added to a new session)

It is true that each UIDL request only accesses one UI and that the locking could therefore be isolated to a UI instance instead of locking the entire VaadinSession. There are however a couple of reasons why it is not implemented this way:

Legacy: Vaadin 6 locked on the Application level which is semantically the same level as VaadinSession in Vaadin 7. Preserving the semantics means that the risk of unpleasant surprises during migration can be reduced and that developers that are used to how Vaadin 6 works does not need to change their way of thinking.

Ease of use: There are lots of ways of accessing different parts of the same VaadinSession but no direct way of accessing data in other VaadinSession instances. By keeping the entire VaadinSession locked when invoking user code, the user can be confident that the code will not cause synchronization issues as long as no “dangerous” mechanisms are used, e.g. sharing data in a static variable or starting a separate Thread.

The KISS principle: Getting everything right with one level of locking is challenging. Getting everything right with multiple levels of locking is even more challenging. Locking on the VaadinSession level is at the very least needed for finding the UI instance when starting to process the request. Just holding on to that locking scheme without introducing another level of granularity reduces the risk of implementation issues that could be very difficult to discover.

It’s good enough: Long running tasks should not be executed on the “UI thread” in any UI framework. We have learned a lot about distributing the work to different threads while developing the push functionality for Vaadin 7.1. The result of this learning might not yet be an integrated part of the core framework, but I would expect a healthy selection of useful add-ons within a foreseeable future and some of those concepts integrated to the core framework in some future release (e.g. Vaadin 7.2)