From 8138be4b0352c5b2409b48ad600941afeed3b20b Mon Sep 17 00:00:00 2001
From: Leif Åstrand <leif@vaadin.com>
Date: Tue, 18 Mar 2014 14:24:04 +0200
Subject: Make removeFromParent throw if the right session is not locked
 (#13473)

Change-Id: Id5ef40db07404d7cb41b26768d18e757b8cae2b3
---
 server/src/com/vaadin/server/VaadinService.java    | 24 +++++++++++++++++++---
 .../ui/AbstractSingleComponentContainer.java       | 15 ++++++++++++++
 2 files changed, 36 insertions(+), 3 deletions(-)

(limited to 'server/src')

diff --git a/server/src/com/vaadin/server/VaadinService.java b/server/src/com/vaadin/server/VaadinService.java
index 6fd0b23f7b..ba1224568a 100644
--- a/server/src/com/vaadin/server/VaadinService.java
+++ b/server/src/com/vaadin/server/VaadinService.java
@@ -1635,14 +1635,32 @@ public abstract class VaadinService implements Serializable {
      *             if the current thread holds the lock for another session
      */
     public static void verifyNoOtherSessionLocked(VaadinSession session) {
-        VaadinSession otherSession = VaadinSession.getCurrent();
-        if (otherSession != null && otherSession != session
-                && otherSession.hasLock()) {
+        if (isOtherSessionLocked(session)) {
             throw new IllegalStateException(
                     "Can't access session while another session is locked by the same thread. This restriction is intended to help avoid deadlocks.");
         }
     }
 
+    /**
+     * Checks whether there might be some {@link VaadinSession} other than the
+     * provided one for which the current thread holds a lock. This method might
+     * not detect all cases where some other session is locked, but it should
+     * cover the most typical situations.
+     * 
+     * @since 7.2
+     * @param session
+     *            the session that is expected to be locked
+     * @return <code>true</code> if another session is also locked by the
+     *         current thread; <code>false</code> if no such session was found
+     */
+    public static boolean isOtherSessionLocked(VaadinSession session) {
+        VaadinSession otherSession = VaadinSession.getCurrent();
+        if (otherSession == null || otherSession == session) {
+            return false;
+        }
+        return otherSession.hasLock();
+    }
+
     /**
      * Verifies that the given CSRF token (aka double submit cookie) is valid
      * for the given session. This is used to protect against Cross Site Request
diff --git a/server/src/com/vaadin/ui/AbstractSingleComponentContainer.java b/server/src/com/vaadin/ui/AbstractSingleComponentContainer.java
index 8ad0d23351..534f01d4eb 100644
--- a/server/src/com/vaadin/ui/AbstractSingleComponentContainer.java
+++ b/server/src/com/vaadin/ui/AbstractSingleComponentContainer.java
@@ -19,6 +19,8 @@ import java.util.Collections;
 import java.util.Iterator;
 
 import com.vaadin.server.ComponentSizeValidator;
+import com.vaadin.server.VaadinService;
+import com.vaadin.server.VaadinSession;
 
 /**
  * Abstract base class for component containers that have only one child
@@ -150,6 +152,19 @@ public abstract class AbstractSingleComponentContainer extends
     // TODO move utility method elsewhere?
     public static void removeFromParent(Component content)
             throws IllegalArgumentException {
+        // Verify the appropriate session is locked
+        UI parentUI = content.getUI();
+        if (parentUI != null) {
+            VaadinSession parentSession = parentUI.getSession();
+            if (parentSession != null && !parentSession.hasLock()) {
+                String message = "Cannot remove from parent when the session is not locked.";
+                if (VaadinService.isOtherSessionLocked(parentSession)) {
+                    message += " Furthermore, there is another locked session, indicating that the component might be about to be moved from one session to another.";
+                }
+                throw new IllegalStateException(message);
+            }
+        }
+
         HasComponents parent = content.getParent();
         if (parent instanceof ComponentContainer) {
             // If the component already has a parent, try to remove it
-- 
cgit v1.2.3