Skip to content

Commit

Permalink
fix!: Prevent deadlock in findOrCreateVaadinSession (#11520) (CP: 2.7) (
Browse files Browse the repository at this point in the history
#12324)

Fixes #12316

BREAKING CHANGE: VaadinService::lockSession now must return the lock, and VaadinService::unlockSession must accept it as a parameter. If you have overridden these methods, you must adapt your implementation accordingly.

Co-authored-by: Tatu Lund <tatu@vaadin.com>
Co-authored-by: Pekka Hyvönen <pekka@vaadin.com>
  • Loading branch information
3 people committed Nov 11, 2021
1 parent eb3207d commit e530e5a
Showing 1 changed file with 27 additions and 7 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -777,13 +777,22 @@ protected Lock getSessionLock(WrappedSession wrappedSession) {
/**
* 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 Flow 2.7.4. 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) {
/*
Expand Down Expand Up @@ -813,21 +822,31 @@ protected void lockSession(WrappedSession wrappedSession) {
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 Flow 2.7.4. 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
* @param lock
* Lock instance to unlock
*/
protected void unlockSession(WrappedSession wrappedSession) {
assert getSessionLock(wrappedSession) != null;
assert ((ReentrantLock) getSessionLock(wrappedSession))
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";
getSessionLock(wrappedSession).unlock();
lock.unlock();
}

private VaadinSession findOrCreateVaadinSession(VaadinRequest request)
Expand All @@ -836,8 +855,9 @@ private VaadinSession findOrCreateVaadinSession(VaadinRequest request)
WrappedSession wrappedSession = getWrappedSession(request,
requestCanCreateSession);

final Lock lock;
try {
lockSession(wrappedSession);
lock = lockSession(wrappedSession);
} catch (IllegalStateException e) {
throw new SessionExpiredException();
}
Expand All @@ -846,7 +866,7 @@ private VaadinSession findOrCreateVaadinSession(VaadinRequest request)
return doFindOrCreateVaadinSession(request,
requestCanCreateSession);
} finally {
unlockSession(wrappedSession);
unlockSession(wrappedSession, lock);
}

}
Expand Down

0 comments on commit e530e5a

Please sign in to comment.