From 1a9c7d1bdb23a7d3ec2a3369d75504b9eebd97e5 Mon Sep 17 00:00:00 2001 From: Artur Signell Date: Thu, 14 Mar 2013 12:43:53 +0200 Subject: [PATCH] Changed VaadinSession locking to be based on a session attribute (#8894) Locking of a VaadinSession is now done in VaadinService using the underlying HTTP session. This enables locking of the VaadinSession even before it has been created. Change-Id: I815d08d1fb74a1d0905c58b190bb10aa2161a834 --- .../src/com/vaadin/server/VaadinService.java | 193 ++++++++++++++++-- .../src/com/vaadin/server/VaadinSession.java | 44 ++-- 2 files changed, 208 insertions(+), 29 deletions(-) diff --git a/server/src/com/vaadin/server/VaadinService.java b/server/src/com/vaadin/server/VaadinService.java index e5d72969c6..d3ac4bb1b7 100644 --- a/server/src/com/vaadin/server/VaadinService.java +++ b/server/src/com/vaadin/server/VaadinService.java @@ -26,12 +26,13 @@ import java.net.URL; import java.util.ArrayList; import java.util.HashMap; import java.util.Locale; +import java.util.concurrent.locks.Lock; +import java.util.concurrent.locks.ReentrantLock; import java.util.logging.Level; import java.util.logging.Logger; import javax.portlet.PortletContext; import javax.servlet.ServletContext; -import javax.servlet.ServletException; import com.vaadin.annotations.PreserveOnRefresh; import com.vaadin.event.EventRouter; @@ -366,6 +367,10 @@ public abstract class VaadinService implements Serializable { /** * Attempts to find a Vaadin service session associated with this request. + *

+ * Handles locking of the session internally to avoid creation of duplicate + * sessions by two threads simultaneously. + *

* * @param request * the request to get a vaadin service session for. @@ -389,9 +394,135 @@ public abstract class VaadinService implements Serializable { return vaadinSession; } + /** + * Associates the given lock with this service and the given wrapped + * session. This method should not be called more than once when the lock is + * initialized for the session. + * + * @see #getSessionLock(WrappedSession) + * @param wrappedSession + * The wrapped session the lock is associated with + * @param lock + * The lock object + */ + private void setSessionLock(WrappedSession wrappedSession, Lock lock) { + assert wrappedSession != null : "Can't set a lock for a null session"; + assert wrappedSession.getAttribute(getLockAttributeName()) == null : "Changing the lock for a session is not allowed"; + + wrappedSession.setAttribute(getLockAttributeName(), lock); + } + + /** + * Returns the name used to store the lock in the HTTP session. + * + * @return The attribute name for the lock + */ + private String getLockAttributeName() { + return getServiceName() + ".lock"; + } + + /** + * Gets the lock instance used to lock the VaadinSession associated with the + * given wrapped session. + *

+ * This method uses the wrapped session instead of VaadinSession to be able + * to lock even before the VaadinSession has been initialized. + *

+ * + * @param wrappedSession + * The wrapped session + * @return A lock instance used for locking access to the wrapped session + */ + protected Lock getSessionLock(WrappedSession wrappedSession) { + Object lock = wrappedSession.getAttribute(getLockAttributeName()); + + if (lock instanceof ReentrantLock) { + return (ReentrantLock) lock; + } + + if (lock == null) { + return null; + } + + throw new RuntimeException( + "Something else than a ReentrantLock was stored in the " + + getLockAttributeName() + " in the session"); + } + + /** + * Locks the given session for this service instance. Typically you want to + * call {@link VaadinSession#lock()} instead of this method. + * + * @param wrappedSession + * The session to lock + */ + protected void lockSession(WrappedSession wrappedSession) { + Lock lock = getSessionLock(wrappedSession); + if (lock == null) { + /* + * No lock found in the session attribute. Ensure only one lock is + * created and used by everybody by doing double checked locking. + * Assumes there is a memory barrier for the attribute (i.e. that + * the CPU flushes its caches and reads the value directly from main + * memory). + */ + synchronized (VaadinService.class) { + lock = getSessionLock(wrappedSession); + if (lock == null) { + lock = new ReentrantLock(); + setSessionLock(wrappedSession, lock); + } + } + } + lock.lock(); + } + + /** + * Releases the lock for the given session for this service instance. + * Typically you want to call {@link VaadinSession#unlock()} instead of this + * method. + * + * @param wrappedSession + * The session 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(); + } + private VaadinSession findOrCreateVaadinSession(VaadinRequest request) throws SessionExpiredException, ServiceException { boolean requestCanCreateSession = requestCanCreateSession(request); + WrappedSession wrappedSession = getWrappedSession(request, + requestCanCreateSession); + + lockSession(wrappedSession); + try { + return doFindOrCreateVaadinSession(request, requestCanCreateSession); + } finally { + unlockSession(wrappedSession); + } + + } + + /** + * Finds or creates a Vaadin session. Assumes necessary synchronization has + * been done by the caller to ensure this is not called simultaneously by + * several threads. + * + * @param request + * @param requestCanCreateSession + * @return + * @throws SessionExpiredException + * @throws ServiceException + */ + private VaadinSession doFindOrCreateVaadinSession(VaadinRequest request, + boolean requestCanCreateSession) throws SessionExpiredException, + ServiceException { + assert ((ReentrantLock) getSessionLock(request.getWrappedSession())) + .isHeldByCurrentThread() : "Session has not been locked by this thread"; /* Find an existing session for this request. */ VaadinSession session = getExistingSession(request, @@ -437,8 +568,21 @@ public abstract class VaadinService implements Serializable { } + /** + * Creates and registers a new VaadinSession for this service. Assumes + * proper locking has been taken care of by the caller. + * + * + * @param request + * The request which triggered session creation. + * @return A new VaadinSession instance + * @throws ServiceException + */ private VaadinSession createAndRegisterSession(VaadinRequest request) throws ServiceException { + assert ((ReentrantLock) getSessionLock(request.getWrappedSession())) + .isHeldByCurrentThread() : "Session has not been locked by this thread"; + VaadinSession session = createVaadinSession(request); VaadinSession.setCurrent(session); @@ -476,12 +620,13 @@ public abstract class VaadinService implements Serializable { } /** - * Creates a new Vaadin service session. + * Creates a new Vaadin session for this service and request * * @param request - * @return - * @throws ServletException - * @throws MalformedURLException + * The request for which to create a VaadinSession + * @return A new VaadinSession + * @throws ServiceException + * */ protected VaadinSession createVaadinSession(VaadinRequest request) throws ServiceException { @@ -509,12 +654,8 @@ public abstract class VaadinService implements Serializable { protected VaadinSession getExistingSession(VaadinRequest request, boolean allowSessionCreation) throws SessionExpiredException { - // Ensures that the session is still valid - final WrappedSession session = request - .getWrappedSession(allowSessionCreation); - if (session == null) { - throw new SessionExpiredException(); - } + final WrappedSession session = getWrappedSession(request, + allowSessionCreation); VaadinSession vaadinSession = VaadinSession .getForSession(this, session); @@ -526,6 +667,28 @@ public abstract class VaadinService implements Serializable { return vaadinSession; } + /** + * Retrieves the wrapped session for the request. + * + * @param request + * The request for which to retrieve a session + * @param requestCanCreateSession + * true to create a new session if one currently does not exist + * @return The retrieved (or created) wrapped session + * @throws SessionExpiredException + * If the request is not associated to a session and new session + * creation is not allowed + */ + private WrappedSession getWrappedSession(VaadinRequest request, + boolean requestCanCreateSession) throws SessionExpiredException { + final WrappedSession session = request + .getWrappedSession(requestCanCreateSession); + if (session == null) { + throw new SessionExpiredException(); + } + return session; + } + /** * Checks whether it's valid to create a new service session as a result of * the given request. @@ -731,8 +894,12 @@ public abstract class VaadinService implements Serializable { // Ensure VaadinServiceSession knows where it's stored if (value instanceof VaadinSession) { VaadinSession serviceSession = (VaadinSession) value; - serviceSession.storeInSession(serviceSession.getService(), - newSession); + VaadinService service = serviceSession.getService(); + // Use the same lock instance in the new session + service.setSessionLock(newSession, + serviceSession.getLockInstance()); + + serviceSession.storeInSession(service, newSession); serviceSession .setAttribute(REINITIALIZING_SESSION_MARKER, null); } diff --git a/server/src/com/vaadin/server/VaadinSession.java b/server/src/com/vaadin/server/VaadinSession.java index 322bfff924..7377e89bcd 100644 --- a/server/src/com/vaadin/server/VaadinSession.java +++ b/server/src/com/vaadin/server/VaadinSession.java @@ -73,8 +73,6 @@ public class VaadinSession implements HttpSessionBindingListener, Serializable { .findMethod(BootstrapListener.class, "modifyBootstrapPage", BootstrapPageResponse.class); - private final Lock lock = new ReentrantLock(); - /** * Configuration for the session. */ @@ -128,6 +126,8 @@ public class VaadinSession implements HttpSessionBindingListener, Serializable { private transient VaadinService service; + private transient Lock lock; + /** * Create a new service session tied to a Vaadin service * @@ -278,16 +278,19 @@ public class VaadinSession implements HttpSessionBindingListener, Serializable { @Deprecated public static VaadinSession getForSession(VaadinService service, WrappedSession underlyingSession) { - Object attribute = underlyingSession.getAttribute(VaadinSession.class - .getName() + "." + service.getServiceName()); - if (attribute instanceof VaadinSession) { - VaadinSession vaadinSession = (VaadinSession) attribute; - vaadinSession.session = underlyingSession; - vaadinSession.service = service; - return vaadinSession; + assert ((ReentrantLock) service.getSessionLock(underlyingSession)) + .isHeldByCurrentThread() : "Cannot retrieve VaadinSession when WrappedSession is not locked for the service"; + VaadinSession vaadinSession = (VaadinSession) underlyingSession + .getAttribute(getSessionAttributeName(service)); + if (vaadinSession == null) { + return null; } - return null; + vaadinSession.session = underlyingSession; + vaadinSession.service = service; + vaadinSession.refreshLock(); + + return vaadinSession; } /** @@ -300,9 +303,11 @@ public class VaadinSession implements HttpSessionBindingListener, Serializable { @Deprecated public void removeFromSession(VaadinService service) { assert (getForSession(service, session) == this); - session.setAttribute( - VaadinSession.class.getName() + "." + service.getServiceName(), - null); + session.setAttribute(getSessionAttributeName(service), null); + } + + private static String getSessionAttributeName(VaadinService service) { + return VaadinSession.class.getName() + "." + service.getServiceName(); } /** @@ -313,10 +318,17 @@ public class VaadinSession implements HttpSessionBindingListener, Serializable { */ @Deprecated public void storeInSession(VaadinService service, WrappedSession session) { - session.setAttribute( - VaadinSession.class.getName() + "." + service.getServiceName(), - this); + session.setAttribute(getSessionAttributeName(service), this); this.session = session; + refreshLock(); + } + + /** + * Updates the transient session lock from VaadinService. + */ + private void refreshLock() { + assert lock == null || lock == service.getSessionLock(session) : "Cannot change the lock from one instance to another"; + lock = service.getSessionLock(session); } public void setCommunicationManager( -- 2.39.5