fix: return immidiately if VaadinSession is not initialized (#11974) (#…

…11986) (#11988) fixes #11961 Co-authored-by: Denis <[email protected]>
vaadin · Oct 6, 2021 · 98846f4 · 98846f4
1 parent ad6e012
commit 98846f4
Show file tree

Hide file tree

Showing 2 changed files with 57 additions and 8 deletions.
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
@@ -174,16 +174,25 @@ public void valueBound(HttpSessionBindingEvent arg0) {
      */
     @Override
     public void valueUnbound(HttpSessionBindingEvent event) {
-        // If we are going to be unbound from the session, the session must be
-        // closing
-        // Notify the service
-        if (service == null) {
+        // The Vaadin session instance may be not yet initialized properly via
+        // {@link #refreshTransients(WrappedSession, VaadinService)}. It happens
+        // when the session is deserialized and container decides that it's
+        // expired. Such session is not known to anyone (except deserializer)
+        // and nothins should be done with it: just return immediately.
+        //
+        // Be aware that not initialized session doesn't have the
+        // correct/expected state: it has no lock, service, session, etc.
+        if (!isInitialized()) {
             getLogger().warn(
                     "A VaadinSession instance not associated to any service is getting unbound. "
                             + "Session destroy events will not be fired and UIs in the session will not get detached. "
                             + "This might happen if a session is deserialized but never used before it expires.");
-        } else if (VaadinService.getCurrentRequest() != null
-                && getCurrent() == this) {
+            return;
+        }
+        // If we are going to be unbound from the session, the session must be
+        // closing
+        // Notify the service
+        if (VaadinService.getCurrentRequest() != null && getCurrent() == this) {
             checkHasLock();
             // Ignore if the session is being moved to a different backing
             // session or if GAEVaadinServlet is doing its normal cleanup.
@@ -631,8 +640,10 @@ public void removeUI(UI ui) {
      * <code>Lock</code> interface than {@link Lock#lock()} and
      * {@link Lock#unlock()}.
      *
-     * @return the <code>Lock</code> that is used for synchronization, never
-     *         <code>null</code>
+     * @return the <code>Lock</code> that is used for synchronization, it's
+     *         never <code>null</code> for the session which is in use, i.e. if
+     *         {@link #refreshTransients(WrappedSession, VaadinService)} has
+     *         been called for it
      * @see #lock()
      * @see Lock
      */
@@ -1094,4 +1105,22 @@ public StreamResourceRegistry getResourceRegistry() {
     public String getCsrfToken() {
         return csrfToken;
     }
+
+    /**
+     * Checks whether the session is properly initialized/in use.
+     * <p>
+     * The session is initialized if
+     * {@link #refreshTransients(WrappedSession, VaadinService)} has been called
+     * for it. If the session is just created or deserialized but not yet in
+     * real use then it's not initialized and it's state is incomplete.
+     * 
+     * @return whether the session is initialized
+     */
+    private boolean isInitialized() {
+        boolean isInitialized = service != null;
+        assert isInitialized
+                || session == null : "The wrapped session must be null if the service is null (which happens after deserialization)";
+        return isInitialized;
+    }
+
 }
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
@@ -473,4 +473,24 @@ protected Lock getSessionLock(WrappedSession wrappedSession) {
         Assert.assertNull(vaadinSession.getSession());
     }
 
+    @Test
+    public void valueUnbound_sessionIsNotInitialized_noAnyInteractions() {
+        VaadinSession session = Mockito.spy(TestVaadinSession.class);
+
+        HttpSessionBindingEvent event = Mockito
+                .mock(HttpSessionBindingEvent.class);
+        session.valueUnbound(null);
+
+        Mockito.verify(session).valueUnbound(null);
+        Mockito.verifyNoInteractions(event);
+        Mockito.verifyNoMoreInteractions(session);
+    }
+
+    public static class TestVaadinSession extends VaadinSession {
+
+        public TestVaadinSession() {
+            super(null);
+        }
+    }
+
 }