From 2b5ba963af81bb8a21f51ab29bd47ecd0054daa6 Mon Sep 17 00:00:00 2001 From: =?utf8?q?Leif=20=C3=85strand?= Date: Tue, 28 May 2013 14:55:35 +0300 Subject: [PATCH] Define how CurrentInstance works with access() (#11897) Change-Id: I7ca62c5706fd37e7c44ed46703bcdce159b367f4 --- .../src/com/vaadin/server/VaadinSession.java | 82 +++++++++++++------ server/src/com/vaadin/ui/UI.java | 11 ++- .../src/com/vaadin/util/CurrentInstance.java | 46 +++++++++-- .../vaadin/tests/components/ui/UiAccess.html | 51 +++++++++++- .../vaadin/tests/components/ui/UiAccess.java | 82 +++++++++++++++++++ 5 files changed, 236 insertions(+), 36 deletions(-) diff --git a/server/src/com/vaadin/server/VaadinSession.java b/server/src/com/vaadin/server/VaadinSession.java index 9625a3f350..eaf611caad 100644 --- a/server/src/com/vaadin/server/VaadinSession.java +++ b/server/src/com/vaadin/server/VaadinSession.java @@ -67,6 +67,35 @@ import com.vaadin.util.ReflectTools; @SuppressWarnings("serial") public class VaadinSession implements HttpSessionBindingListener, Serializable { + private class FutureAccess extends FutureTask { + /** + * Snapshot of all non-inheritable current instances at the time this + * object was created. + */ + private final Map, CurrentInstance> instances = CurrentInstance + .getInstances(true); + + public FutureAccess(Runnable arg0) { + super(arg0, null); + } + + @Override + public Void get() throws InterruptedException, ExecutionException { + /* + * Help the developer avoid programming patterns that cause + * deadlocks unless implemented very carefully. get(long, TimeUnit) + * does not have the same detection since a sensible timeout should + * avoid completely locking up the application. + * + * Even though no deadlock could occur after the runnable has been + * run, the check is always done as the deterministic behavior makes + * it easier to detect potential problems. + */ + VaadinService.verifyNoOtherSessionLocked(VaadinSession.this); + return super.get(); + } + } + /** * The name of the parameter that is by default used in e.g. web.xml to * define the name of the default {@link UI} class. @@ -140,7 +169,7 @@ public class VaadinSession implements HttpSessionBindingListener, Serializable { * session is serialized as long as it doesn't happen while some other * thread has the lock. */ - private transient final ConcurrentLinkedQueue> pendingAccessQueue = new ConcurrentLinkedQueue>(); + private transient final ConcurrentLinkedQueue pendingAccessQueue = new ConcurrentLinkedQueue(); /** * Create a new service session tied to a Vaadin service @@ -1152,9 +1181,13 @@ public class VaadinSession implements HttpSessionBindingListener, Serializable { *

* Please note that the runnable might be invoked on a different thread or * later on the current thread, which means that custom thread locals might - * not have the expected values when the runnable is executed. The session - * and other thread locals provided by Vaadin are set properly before - * executing the runnable. + * not have the expected values when the runnable is executed. Inheritable + * values in {@link CurrentInstance} will have the same values as when this + * method was invoked. {@link VaadinSession#getCurrent()} and + * {@link VaadinService#getCurrent()} are set according to this session + * before executing the runnable. Non-inheritable CurrentInstance values + * including {@link VaadinService#getCurrentRequest()} and + * {@link VaadinService#getCurrentResponse()} will not be defined. *

*

* The returned future can be used to check for task completion and to @@ -1176,23 +1209,7 @@ public class VaadinSession implements HttpSessionBindingListener, Serializable { * cancel the task */ public Future access(Runnable runnable) { - FutureTask future = new FutureTask(runnable, null) { - @Override - public Void get() throws InterruptedException, ExecutionException { - /* - * Help the developer avoid programming patterns that cause - * deadlocks unless implemented very carefully. get(long, - * TimeUnit) does not have the same detection since a sensible - * timeout should avoid completely locking up the application. - * - * Even though no deadlock could occur after the runnable has - * been run, the check is always done as the deterministic - * behavior makes it easier to detect potential problems. - */ - VaadinService.verifyNoOtherSessionLocked(VaadinSession.this); - return super.get(); - } - }; + FutureAccess future = new FutureAccess(runnable); pendingAccessQueue.add(future); /* @@ -1233,11 +1250,26 @@ public class VaadinSession implements HttpSessionBindingListener, Serializable { public void runPendingAccessTasks() { assert hasLock(); - FutureTask pendingAccess; - while ((pendingAccess = pendingAccessQueue.poll()) != null) { - if (!pendingAccess.isCancelled()) { - accessSynchronously(pendingAccess); + if (pendingAccessQueue.isEmpty()) { + return; + } + + Map, CurrentInstance> oldInstances = CurrentInstance + .getInstances(false); + + FutureAccess pendingAccess; + try { + while ((pendingAccess = pendingAccessQueue.poll()) != null) { + if (!pendingAccess.isCancelled()) { + CurrentInstance.clearAll(); + CurrentInstance + .restoreThreadLocals(pendingAccess.instances); + accessSynchronously(pendingAccess); + } } + } finally { + CurrentInstance.clearAll(); + CurrentInstance.restoreThreadLocals(oldInstances); } } diff --git a/server/src/com/vaadin/ui/UI.java b/server/src/com/vaadin/ui/UI.java index bbc1d0cd33..0b21112c97 100644 --- a/server/src/com/vaadin/ui/UI.java +++ b/server/src/com/vaadin/ui/UI.java @@ -1185,9 +1185,14 @@ public abstract class UI extends AbstractSingleComponentContainer implements *

* Please note that the runnable might be invoked on a different thread or * later on the current thread, which means that custom thread locals might - * not have the expected values when the runnable is executed. The UI and - * other thread locals provided by Vaadin are set properly before executing - * the runnable. + * not have the expected values when the runnable is executed. Inheritable + * values in {@link CurrentInstance} will have the same values as when this + * method was invoked. {@link UI#getCurrent()}, + * {@link VaadinSession#getCurrent()} and {@link VaadinService#getCurrent()} + * are set according to this UI before executing the runnable. + * Non-inheritable CurrentInstance values including + * {@link VaadinService#getCurrentRequest()} and + * {@link VaadinService#getCurrentResponse()} will not be defined. *

*

* The returned future can be used to check for task completion and to diff --git a/server/src/com/vaadin/util/CurrentInstance.java b/server/src/com/vaadin/util/CurrentInstance.java index 0854d422fd..8e6ef8266e 100644 --- a/server/src/com/vaadin/util/CurrentInstance.java +++ b/server/src/com/vaadin/util/CurrentInstance.java @@ -17,33 +17,35 @@ package com.vaadin.util; import java.io.Serializable; +import java.util.Collections; import java.util.HashMap; import java.util.Map; import java.util.Map.Entry; -import com.vaadin.server.VaadinPortlet; import com.vaadin.server.VaadinRequest; import com.vaadin.server.VaadinResponse; import com.vaadin.server.VaadinService; -import com.vaadin.server.VaadinServlet; import com.vaadin.server.VaadinSession; import com.vaadin.ui.UI; /** - * Keeps track of various thread local instances used by the framework. + * Keeps track of various current instances for the current thread. All the + * instances are automatically cleared after handling a request from the client + * to avoid leaking memory. The inheritable values are also maintained when + * execution is moved to another thread, both when a new thread is created and + * when {@link VaadinSession#access(Runnable)} or {@link UI#access(Runnable)} is + * used. *

* Currently the framework uses the following instances: *

*

- * Inheritable: {@link UI}, {@link VaadinPortlet}, {@link VaadinService}, - * {@link VaadinServlet}, {@link VaadinSession}. + * Inheritable: {@link UI}, {@link VaadinService}, {@link VaadinSession}. *

*

* Non-inheritable: {@link VaadinRequest}, {@link VaadinResponse}. *

* * @author Vaadin Ltd - * @version @VERSION@ * @since 7.0.0 */ public class CurrentInstance implements Serializable { @@ -115,7 +117,9 @@ public class CurrentInstance implements Serializable { /** * Sets the current inheritable instance of the given type. A current - * instance that is inheritable will be available for child threads. + * instance that is inheritable will be available for child threads and in + * code run by {@link VaadinSession#access(Runnable)} and + * {@link UI#access(Runnable)}. * * @see #set(Class, Object) * @see InheritableThreadLocal @@ -183,6 +187,34 @@ public class CurrentInstance implements Serializable { } } + /** + * Gets the currently set instances so that they can later be restored using + * {@link #restoreThreadLocals(Map)}. + * + * @since 7.1 + * + * @param onlyInheritable + * true if only the inheritable instances should be + * included; false to get all instances. + * @return a map containing the current instances + */ + public static Map, CurrentInstance> getInstances( + boolean onlyInheritable) { + Map, CurrentInstance> map = instances.get(); + if (map == null) { + return Collections.emptyMap(); + } else { + Map, CurrentInstance> copy = new HashMap, CurrentInstance>(); + for (Class c : map.keySet()) { + CurrentInstance ci = map.get(c); + if (ci.inheritable || !onlyInheritable) { + copy.put(c, ci); + } + } + return copy; + } + } + /** * Sets thread locals for the UI and all related classes * diff --git a/uitest/src/com/vaadin/tests/components/ui/UiAccess.html b/uitest/src/com/vaadin/tests/components/ui/UiAccess.html index 664b15c16f..8ae2f2f48e 100644 --- a/uitest/src/com/vaadin/tests/components/ui/UiAccess.html +++ b/uitest/src/com/vaadin/tests/components/ui/UiAccess.html @@ -121,7 +121,56 @@ vaadin=runcomvaadintestscomponentsuiUiAccess::PID_SLog_row_0 2. I was interrupted - + + click + vaadin=runcomvaadintestscomponentsuiUiAccess::/VVerticalLayout[0]/Slot[2]/VVerticalLayout[0]/Slot[5]/VButton[0]/domChild[0]/domChild[0] + + + + assertText + vaadin=runcomvaadintestscomponentsuiUiAccess::PID_SLog_row_3 + 0. accessSynchronously has request? true + + + assertText + vaadin=runcomvaadintestscomponentsuiUiAccess::PID_SLog_row_2 + 1. Test value in accessSynchronously: Set before accessSynchronosly + + + assertText + vaadin=runcomvaadintestscomponentsuiUiAccess::PID_SLog_row_1 + 2. has request after accessSynchronously? true + + + assertText + vaadin=runcomvaadintestscomponentsuiUiAccess::PID_SLog_row_0 + 3. Test value after accessSynchornously: Set in accessSynchronosly + + + click + vaadin=runcomvaadintestscomponentsuiUiAccess::/VVerticalLayout[0]/Slot[2]/VVerticalLayout[0]/Slot[6]/VButton[0]/domChild[0]/domChild[0] + + + + assertText + vaadin=runcomvaadintestscomponentsuiUiAccess::PID_SLog_row_3 + 0. access has request? false + + + assertText + vaadin=runcomvaadintestscomponentsuiUiAccess::PID_SLog_row_2 + 1. Test value in access: Set before access + + + assertText + vaadin=runcomvaadintestscomponentsuiUiAccess::PID_SLog_row_1 + 2. has request after access? true + + + assertText + vaadin=runcomvaadintestscomponentsuiUiAccess::PID_SLog_row_0 + 3. Test value after access: Set before run pending + diff --git a/uitest/src/com/vaadin/tests/components/ui/UiAccess.java b/uitest/src/com/vaadin/tests/components/ui/UiAccess.java index c68da6ee54..c39f65243d 100644 --- a/uitest/src/com/vaadin/tests/components/ui/UiAccess.java +++ b/uitest/src/com/vaadin/tests/components/ui/UiAccess.java @@ -26,11 +26,25 @@ import com.vaadin.server.VaadinService; import com.vaadin.tests.components.AbstractTestUIWithLog; import com.vaadin.ui.Button; import com.vaadin.ui.Button.ClickEvent; +import com.vaadin.util.CurrentInstance; public class UiAccess extends AbstractTestUIWithLog { private Future checkFromBeforeClientResponse; + private class CurrentInstanceTestType { + private String value; + + public CurrentInstanceTestType(String value) { + this.value = value; + } + + @Override + public String toString() { + return value; + } + } + @Override protected void setup(VaadinRequest request) { addComponent(new Button("Access from UI thread", @@ -211,6 +225,74 @@ public class UiAccess extends AbstractTestUIWithLog { }.start(); } })); + addComponent(new Button("CurrentInstance accessSynchronously values", + new Button.ClickListener() { + @Override + public void buttonClick(ClickEvent event) { + log.clear(); + // accessSynchronously should maintain values + CurrentInstance.set(CurrentInstanceTestType.class, + new CurrentInstanceTestType( + "Set before accessSynchronosly")); + accessSynchronously(new Runnable() { + @Override + public void run() { + log.log("accessSynchronously has request? " + + (VaadinService.getCurrentRequest() != null)); + log.log("Test value in accessSynchronously: " + + CurrentInstance + .get(CurrentInstanceTestType.class)); + CurrentInstance.set( + CurrentInstanceTestType.class, + new CurrentInstanceTestType( + "Set in accessSynchronosly")); + } + }); + log.log("has request after accessSynchronously? " + + (VaadinService.getCurrentRequest() != null)); + log("Test value after accessSynchornously: " + + CurrentInstance + .get(CurrentInstanceTestType.class)); + } + })); + addComponent(new Button("CurrentInstance access values", + new Button.ClickListener() { + @Override + public void buttonClick(ClickEvent event) { + log.clear(); + // accessSynchronously should maintain values + CurrentInstance + .setInheritable(CurrentInstanceTestType.class, + new CurrentInstanceTestType( + "Set before access")); + access(new Runnable() { + @Override + public void run() { + log.log("access has request? " + + (VaadinService.getCurrentRequest() != null)); + log.log("Test value in access: " + + CurrentInstance + .get(CurrentInstanceTestType.class)); + CurrentInstance.setInheritable( + CurrentInstanceTestType.class, + new CurrentInstanceTestType( + "Set in access")); + } + }); + CurrentInstance.setInheritable( + CurrentInstanceTestType.class, + new CurrentInstanceTestType( + "Set before run pending")); + + getSession().runPendingAccessTasks(); + + log.log("has request after access? " + + (VaadinService.getCurrentRequest() != null)); + log("Test value after access: " + + CurrentInstance + .get(CurrentInstanceTestType.class)); + } + })); } @Override -- 2.39.5