From 98846f44b24f52f1825428ed2a6c0dc66e2968ee Mon Sep 17 00:00:00 2001 From: Vaadin Bot Date: Wed, 6 Oct 2021 13:00:47 +0300 Subject: [PATCH] fix: return immidiately if VaadinSession is not initialized (#11974) (#11986) (#11988) fixes #11961 Co-authored-by: Denis --- .../com/vaadin/flow/server/VaadinSession.java | 45 +++++++++++++++---- .../vaadin/flow/server/VaadinSessionTest.java | 20 +++++++++ 2 files changed, 57 insertions(+), 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 index 3545c565104..a0bf7f1bfd1 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 @@ -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) { * Lock interface than {@link Lock#lock()} and * {@link Lock#unlock()}. * - * @return the Lock that is used for synchronization, never - * null + * @return the Lock that is used for synchronization, it's + * never null 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. + *

+ * 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 index 62956e352c1..cb9683b9302 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 @@ -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); + } + } + }