fix: Prevent deadlock in findOrCreateVaadinSession (#12355)

It is possible findOrCreateVaadinSession leaves session locked if HTTP session is invalidated in an another thread between lockSession and unlockSession calls. This PR changes the implementation so that this can no longer happen.

fixes: #12352

1 files changed, 30 insertions, 11 deletions

diff --git a/server/src/main/java/com/vaadin/server/VaadinService.java b/server/src/main/java/com/vaadin/server/VaadinService.java
index 3867696075..888b82a3cb 100644
--- a/server/src/main/java/com/vaadin/server/VaadinService.java
+++ b/server/src/main/java/com/vaadin/server/VaadinService.java
@@ -677,14 +677,23 @@ public abstract class VaadinService implements Serializable {
     /**
      * Locks the given session for this service instance. Typically you want to
      * call {@link VaadinSession#lock()} instead of this method.
-     *
+     * <p>
+     * Note: The method and its signature has been changed to return lock 
+     * instance in Vaadin 8.14.0. If you have overriden this method, you need
+     * to update your implementation.
+     * <p>
+     * Note: Overriding this method is not recommended, for custom lock storage
+     * strategy override {@link #getSessionLock(WrappedSession)} and
+     * {@link #setSessionLock(WrappedSession,Lock)} instead.
+     * 
      * @param wrappedSession
      *            The session to lock
-     *
+     * @return Lock instance
+     * 
      * @throws IllegalStateException
      *             if the session is invalidated before it can be locked
      */
-    protected void lockSession(WrappedSession wrappedSession) {
+    protected Lock lockSession(WrappedSession wrappedSession) {
         Lock lock = getSessionLock(wrappedSession);
         if (lock == null) {
             /*
@@ -714,21 +723,30 @@ public abstract class VaadinService implements Serializable {
             lock.unlock();
             throw e;
         }
+        return lock;
     }
 
     /**
      * Releases the lock for the given session for this service instance.
      * Typically you want to call {@link VaadinSession#unlock()} instead of this
      * method.
+     * <p>
+     * Note: The method and its signature has been changed to get lock instance
+     * as parameter in Vaadin 8.14.0. If you have overriden this method, you need
+     * to update your implementation.
+     * <p>
+     * Note: Overriding this method is not recommended, for custom lock storage 
+     * strategy override {@link #getSessionLock(WrappedSession)} and
+     * {@link #setSessionLock(WrappedSession,Lock)} instead.
      *
      * @param wrappedSession
-     *            The session to unlock
+     *            The session to unlock, used only with assert
+     * @param lock
+     *            Lock instance to unlock
      */
-    protected void unlockSession(WrappedSession wrappedSession) {
-        assert getSessionLock(wrappedSession) != null;
-        assert ((ReentrantLock) getSessionLock(wrappedSession))
-                .isHeldByCurrentThread() : "Trying to unlock the session but it has not been locked by this thread";
-        getSessionLock(wrappedSession).unlock();
+    protected void unlockSession(WrappedSession wrappedSession, Lock lock) {
+        assert ((ReentrantLock) lock).isHeldByCurrentThread() : "Trying to unlock the session but it has not been locked by this thread";
+        lock.unlock();
     }
 
     private VaadinSession findOrCreateVaadinSession(VaadinRequest request)
@@ -737,8 +755,9 @@ public abstract class VaadinService implements Serializable {
         WrappedSession wrappedSession = getWrappedSession(request,
                 requestCanCreateSession);
 
+        final Lock lock;
         try {
-            lockSession(wrappedSession);
+            lock = lockSession(wrappedSession);
         } catch (IllegalStateException e) {
             throw new SessionExpiredException();
         }
@@ -747,7 +766,7 @@ public abstract class VaadinService implements Serializable {
             return doFindOrCreateVaadinSession(request,
                     requestCanCreateSession);
         } finally {
-            unlockSession(wrappedSession);
+            unlockSession(wrappedSession, lock);
         }
 
     }