fix: clear CurrentInstance before invoking new session tasks (#6349) (#…

…20255) (#20280)

Commands enqueued by VaadinSession.access() in general have nothing to do with each other. The only thing they have in common is they share the same VaadinSession (and, by implication, VaadinService).

Therefore, if command №1 invoked UI.setCurrent() and command №2 invokes UI.getCurrent(), command №2 should read null, not the random UI from command №1 that it has nothing to do with.

Fixes #6349

Co-authored-by: Archie L. Cobbs <[email protected]>

flow-server/src/main/java/com/vaadin/flow/server/VaadinService.java

@@ -2093,11 +2093,12 @@ public void runPendingAccessTasks(VaadinSession session) {
         // Dump all current instances, not only the ones dumped by setCurrent
         Map<Class<?>, CurrentInstance> oldInstances = CurrentInstance
                 .getInstances();
-        CurrentInstance.setCurrent(session);
         try {
             while ((pendingAccess = session.getPendingAccessQueue()
                     .poll()) != null) {
                 if (!pendingAccess.isCancelled()) {
+                    CurrentInstance.clearAll();
+                    CurrentInstance.setCurrent(session);
                     pendingAccess.run();
 
                     try {

flow-server/src/test/java/com/vaadin/flow/server/VaadinSessionTest.java

@@ -29,6 +29,8 @@
 import java.util.Set;
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.atomic.AtomicBoolean;
+import java.util.concurrent.atomic.AtomicInteger;
+import java.util.concurrent.atomic.AtomicReference;
 import java.util.concurrent.locks.Lock;
 import java.util.concurrent.locks.ReentrantLock;
 import java.util.stream.Collectors;
@@ -189,6 +191,69 @@ public Map<String, String[]> getParameterMap() {
 
     }
 
+    /**
+     * Test for issue #6349
+     */
+    @Test
+    public void testCurrentInstancePollution() throws InterruptedException {
+
+        // Create a sync object
+        final AtomicInteger state = new AtomicInteger();
+
+        // For sleeping while we wait
+        final Runnable napper = () -> {
+            try {
+                Thread.sleep(10);
+            } catch (InterruptedException e) {
+                return;
+            }
+        };
+
+        // We need to unlock session before running this test
+        session.unlock();
+        try {
+
+            // Create a thread that holds the session lock until we tell it to
+            // unlock; this will cause VaadinSession.access() to enqueue tasks
+            // instead of running them immediately.
+            Thread thread = new Thread(() -> {
+                session.accessSynchronously(() -> {
+                    state.incrementAndGet();
+                    while (state.get() == 1)
+                        napper.run();
+                });
+            });
+
+            // Start thread and wait for it to grab the lock
+            thread.start();
+            while (state.get() == 0)
+                napper.run();
+
+            // Enqueue two session commands
+            session.access(() -> {
+                UI.setCurrent(ui); // command #1 sets current UI
+                state.incrementAndGet();
+            });
+            final AtomicReference<UI> uiRef = new AtomicReference<>();
+            session.access(() -> {
+                uiRef.set(UI.getCurrent()); // command #2 reads current UI
+                state.incrementAndGet();
+            });
+
+            // Release the session lock, which will run the queue
+            state.incrementAndGet();
+
+            // Wait for enqueued tasks to complete
+            while (state.get() < 4)
+                napper.run();
+
+            // Command #2 should not have seen command #1's UI
+            Assert.assertNull(uiRef.get());
+        } finally {
+            session.lock();
+        }
+    }
+
     /**
      * This reproduces #14452 situation with deadlock - see diagram
      */