vaadin / flow

Vaadin Flow is a Java framework binding Vaadin web components to Java. This is part of Vaadin 10+.
Apache License 2.0
622 stars 166 forks source link

Make explicit the fact that VaadinSession.access() guarantees Commands are executed in order #13161

Open archiecobbs opened 2 years ago

archiecobbs commented 2 years ago

Describe your motivation

The way VaadinSession.access() is implemented, it guarantees that the given commands are executed in the same order that they are given to this method.

This is because if the session is already locked, the commands are invoked synchronously, while if the session is not locked, they are added to a Queue which of course has FIFO semantics.

This is very good!!

And this is a very important property to preserve because there is undoubtedly a lot of code out there (including my own) that relies on it. For example, so that certain notifications are delivered in the expected order, etc.

I would even venture to guess there is a lot of code out there that unwittingly relies on this.

The problem is that this guarantee is not actually documented or made explicit in any way.

So in theory there is a bunch of code out there that has a bug, because it's relying on undocumented behavior.

The solution: document this behavior to remove all doubt - and make this method even more useful, since we can now sleep a night knowing somebody won't accidentally screw up in-order processing in the future.

Describe the solution you'd like

Simply apply this patch:

diff --git a/flow-server/src/main/java/com/vaadin/flow/server/VaadinSession.java b/flow-server/src/main/java/com/vaadin/flow/server/VaadinSession.java
index 47361da5ba..8c1e0c5e68 100644
--- a/flow-server/src/main/java/com/vaadin/flow/server/VaadinSession.java
+++ b/flow-server/src/main/java/com/vaadin/flow/server/VaadinSession.java
@@ -1002,6 +1002,9 @@ public class VaadinSession implements HttpSessionBindingListener, Serializable {
      * cancel the task. To help avoiding deadlocks, {@link Future#get()} throws
      * an exception if it is detected that the current thread holds the lock for
      * some other session.
+     * <p>
+     * If this method is invoked multiple times, the corresponding commands are
+     * guaranteed to be invoked in that same order.
      *
      * @param command
      *            the command which accesses the session

Describe alternatives you've considered

Doing nothing.

Additional context

Ask me if you need further explanation. FWIW this seems "obvious" to me.

mperktold commented 2 months ago

This is because if the session is already locked, the commands are invoked synchronously, while if the session is not locked, they are added to a Queue which of course has FIFO semantics.

It's actually the other way around (kind of): If the session is already locked by any thread, the command is added to the queue, so that thread will execute this command before releasing the lock. If the session is not locked, the caller thread will obtain the lock and run the command right away.

I think you're still right in that this guarantees execution in order, but only among the commands added by the same thread.