From 875e768e7be0500b0991e8bff68d1e93bfbf171d Mon Sep 17 00:00:00 2001
From: Vaadin Bot <vaadin-bot@users.noreply.github.com>
Date: Fri, 18 Oct 2024 10:16:23 +0200
Subject: [PATCH] fix: clear CurrentInstance before invoking new session tasks
 (#6349) (#20255) (#20281)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

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 <archie.cobbs@gmail.com>
---
 .../com/vaadin/flow/server/VaadinService.java |  3 +-
 .../vaadin/flow/server/VaadinSessionTest.java | 65 +++++++++++++++++++
 2 files changed, 67 insertions(+), 1 deletion(-)

diff --git a/flow-server/src/main/java/com/vaadin/flow/server/VaadinService.java b/flow-server/src/main/java/com/vaadin/flow/server/VaadinService.java
index 1959f249ec8..a1e41d15fcc 100644
--- a/flow-server/src/main/java/com/vaadin/flow/server/VaadinService.java
+++ b/flow-server/src/main/java/com/vaadin/flow/server/VaadinService.java
@@ -2035,11 +2035,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 {
diff --git a/flow-server/src/test/java/com/vaadin/flow/server/VaadinSessionTest.java b/flow-server/src/test/java/com/vaadin/flow/server/VaadinSessionTest.java
index 47b81d7cd6f..6fed0d73d8b 100644
--- a/flow-server/src/test/java/com/vaadin/flow/server/VaadinSessionTest.java
+++ b/flow-server/src/test/java/com/vaadin/flow/server/VaadinSessionTest.java
@@ -23,6 +23,8 @@
 import java.util.concurrent.Future;
 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;
@@ -161,6 +163,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
      */