From c8cee295021dcd33982217e3ad1c374bfca63a29 Mon Sep 17 00:00:00 2001 From: =?utf8?q?Johannes=20Dahlstr=C3=B6m?= Date: Fri, 24 Aug 2012 11:31:35 +0300 Subject: [PATCH] Add support for heartbeat not extending session (#9266) --- .../gwt/client/ApplicationConnection.java | 1 - server/src/com/vaadin/Application.java | 90 +++++++++++++++---- .../vaadin/service/ApplicationContext.java | 6 ++ .../terminal/DeploymentConfiguration.java | 20 ++++- .../server/AbstractCommunicationManager.java | 4 +- .../AbstractDeploymentConfiguration.java | 13 +++ .../vaadin/terminal/gwt/server/Constants.java | 2 +- .../server/PortletApplicationContext2.java | 5 ++ .../gwt/server/WebApplicationContext.java | 4 + server/src/com/vaadin/ui/Root.java | 35 ++++++-- 10 files changed, 147 insertions(+), 33 deletions(-) diff --git a/client/src/com/vaadin/terminal/gwt/client/ApplicationConnection.java b/client/src/com/vaadin/terminal/gwt/client/ApplicationConnection.java index b4635c6d80..299278269c 100644 --- a/client/src/com/vaadin/terminal/gwt/client/ApplicationConnection.java +++ b/client/src/com/vaadin/terminal/gwt/client/ApplicationConnection.java @@ -2611,7 +2611,6 @@ public class ApplicationConnection { * TODO: Improved error handling, like in doUidlRequest(). * * @see #scheduleHeartbeat() - * @see com.vaadin.ui.Root#heartbeat() */ protected void sendHeartbeat() { final String uri = addGetParameters( diff --git a/server/src/com/vaadin/Application.java b/server/src/com/vaadin/Application.java index c832645405..8efcadb667 100644 --- a/server/src/com/vaadin/Application.java +++ b/server/src/com/vaadin/Application.java @@ -2424,30 +2424,28 @@ public class Application implements Terminal.ErrorListener, Serializable { } /** - * Removes all those roots from the application whose last heartbeat - * occurred more than {@link #getHeartbeatTimeout()} seconds ago. Close - * events are fired for the removed roots. If - * getHeartbeatTimeout() returns a nonpositive number, no - * cleanup is performed. + * Removes all those roots from the application for whom + * {@link #isRootAlive} returns false. Close events are fired for the + * removed roots. *

* Called by the framework at the end of every request. * * @see Root.CloseEvent * @see Root.CloseListener - * @see #getHeartbeatTimeout() + * @see #isRootAlive(Root) * * @since 7.0.0 */ public void closeInactiveRoots() { - if (getHeartbeatTimeout() > 0) { - long now = System.currentTimeMillis(); - for (Iterator i = roots.values().iterator(); i.hasNext();) { - Root root = i.next(); - if (now - root.getLastHeartbeat() > 1000 * getHeartbeatTimeout()) { - i.remove(); - retainOnRefreshRoots.values().remove(root.getRootId()); - root.fireCloseEvent(); - } + for (Iterator i = roots.values().iterator(); i.hasNext();) { + Root root = i.next(); + if (!isRootAlive(root)) { + i.remove(); + retainOnRefreshRoots.values().remove(root.getRootId()); + root.fireCloseEvent(); + getLogger().info( + "Closed root #" + root.getRootId() + + " due to inactivity"); } } } @@ -2456,15 +2454,69 @@ public class Application implements Terminal.ErrorListener, Serializable { * Returns the number of seconds that must pass without a valid heartbeat or * UIDL request being received from a root before that root is removed from * the application. This is a lower bound; it might take longer to close an - * inactive root. + * inactive root. Returns a negative number if heartbeat is disabled and + * timeout never occurs. + * + * @see #getUidlRequestTimeout() + * @see #closeInactiveRoots() + * @see DeploymentConfiguration#getHeartbeatInterval() * * @since 7.0.0 * - * @return The heartbeat timeout in seconds or a nonpositive number if - * timeout never occurs. + * @return The heartbeat timeout in seconds or a negative number if timeout + * never occurs. */ - public int getHeartbeatTimeout() { + protected int getHeartbeatTimeout() { // Permit three missed heartbeats before closing the root return (int) (configuration.getHeartbeatInterval() * (3.1)); } + + /** + * Returns the number of seconds that must pass without a valid UIDL request + * being received from a root before the root is removed from the + * application, even though heartbeat requests are received. This is a lower + * bound; it might take longer to close an inactive root. Returns a negative + * number if + *

+ * This timeout only has effect if cleanup of inactive roots is enabled; + * otherwise heartbeat requests are enough to extend root lifetime + * indefinitely. + * + * @see DeploymentConfiguration#isIdleRootCleanupEnabled() + * @see #getHeartbeatTimeout() + * @see #closeInactiveRoots() + * + * @since 7.0.0 + * + * @return The UIDL request timeout in seconds, or a negative number if + * timeout never occurs. + */ + protected int getUidlRequestTimeout() { + return configuration.isIdleRootCleanupEnabled() ? getContext() + .getMaxInactiveInterval() : -1; + } + + /** + * Returns whether the given root is alive (the client-side actively + * communicates with the server) or whether it can be removed from the + * application and eventually collected. + * + * @since 7.0.0 + * + * @param root + * The Root whose status to check + * @return true if the root is alive, false if it could be removed. + */ + protected boolean isRootAlive(Root root) { + long now = System.currentTimeMillis(); + if (getHeartbeatTimeout() >= 0 + && now - root.getLastHeartbeatTime() > 1000 * getHeartbeatTimeout()) { + return false; + } + if (getUidlRequestTimeout() >= 0 + && now - root.getLastUidlRequestTime() > 1000 * getUidlRequestTimeout()) { + return false; + } + return true; + } } diff --git a/server/src/com/vaadin/service/ApplicationContext.java b/server/src/com/vaadin/service/ApplicationContext.java index c6116d6e73..55495dcd5c 100644 --- a/server/src/com/vaadin/service/ApplicationContext.java +++ b/server/src/com/vaadin/service/ApplicationContext.java @@ -79,6 +79,12 @@ public interface ApplicationContext extends Serializable { */ public void removeTransactionListener(TransactionListener listener); + /** + * Returns the time between requests, in seconds, before this context is + * invalidated. A negative time indicates the context should never timeout. + */ + public int getMaxInactiveInterval(); + /** * Generate a URL that can be used as the relative location of e.g. an * {@link ApplicationResource}. diff --git a/server/src/com/vaadin/terminal/DeploymentConfiguration.java b/server/src/com/vaadin/terminal/DeploymentConfiguration.java index 8d5686ebbd..d3fd4567f2 100644 --- a/server/src/com/vaadin/terminal/DeploymentConfiguration.java +++ b/server/src/com/vaadin/terminal/DeploymentConfiguration.java @@ -23,6 +23,7 @@ import java.util.Properties; import javax.portlet.PortletContext; import javax.servlet.ServletContext; +import com.vaadin.service.ApplicationContext; import com.vaadin.terminal.gwt.server.AddonContext; import com.vaadin.terminal.gwt.server.AddonContextListener; @@ -162,11 +163,26 @@ public interface DeploymentConfiguration extends Serializable { /** * Returns the number of seconds between heartbeat requests of a root, or a - * non-negative number if heartbeat is disabled. + * non-positive number if heartbeat is disabled. * * @since 7.0.0 * - * @return + * @return The time between heartbeats. */ public int getHeartbeatInterval(); + + /** + * Returns whether roots that have no other activity than heartbeat requests + * should be closed after they have been idle the maximum inactivity time + * enforced by the session. + * + * @see ApplicationContext#getMaxInactiveInterval() + * + * @since 7.0.0 + * + * @return True if roots receiving only heartbeat requests are eventually + * closed; false if heartbeat requests extend root lifetime + * indefinitely. + */ + public boolean isIdleRootCleanupEnabled(); } diff --git a/server/src/com/vaadin/terminal/gwt/server/AbstractCommunicationManager.java b/server/src/com/vaadin/terminal/gwt/server/AbstractCommunicationManager.java index 8bb14af45f..b5ea6a8735 100644 --- a/server/src/com/vaadin/terminal/gwt/server/AbstractCommunicationManager.java +++ b/server/src/com/vaadin/terminal/gwt/server/AbstractCommunicationManager.java @@ -581,7 +581,7 @@ public abstract class AbstractCommunicationManager implements Serializable { } // Keep the root alive - root.heartbeat(); + root.setLastUidlRequestTime(System.currentTimeMillis()); // Change all variables based on request parameters if (!handleVariables(request, response, callback, application, root)) { @@ -2683,7 +2683,7 @@ public abstract class AbstractCommunicationManager implements Serializable { // null-check below handles this as well } if (root != null) { - root.heartbeat(); + root.setLastHeartbeatTime(System.currentTimeMillis()); } else { response.sendError(HttpServletResponse.SC_NOT_FOUND, "Root not found"); diff --git a/server/src/com/vaadin/terminal/gwt/server/AbstractDeploymentConfiguration.java b/server/src/com/vaadin/terminal/gwt/server/AbstractDeploymentConfiguration.java index 1895560209..30ba82f664 100644 --- a/server/src/com/vaadin/terminal/gwt/server/AbstractDeploymentConfiguration.java +++ b/server/src/com/vaadin/terminal/gwt/server/AbstractDeploymentConfiguration.java @@ -34,6 +34,7 @@ public abstract class AbstractDeploymentConfiguration implements private boolean xsrfProtectionEnabled; private int resourceCacheTime; private int heartbeatInterval; + private boolean idleRootCleanupEnabled; public AbstractDeploymentConfiguration(Class systemPropertyBaseClass, Properties applicationProperties) { @@ -44,6 +45,7 @@ public abstract class AbstractDeploymentConfiguration implements checkXsrfProtection(); checkResourceCacheTime(); checkHeartbeatInterval(); + checkIdleRootCleanup(); } @Override @@ -204,6 +206,11 @@ public abstract class AbstractDeploymentConfiguration implements return heartbeatInterval; } + @Override + public boolean isIdleRootCleanupEnabled() { + return idleRootCleanupEnabled; + } + /** * Log a warning if Vaadin is not running in production mode. */ @@ -256,6 +263,12 @@ public abstract class AbstractDeploymentConfiguration implements } } + private void checkIdleRootCleanup() { + idleRootCleanupEnabled = getApplicationOrSystemProperty( + Constants.SERVLET_PARAMETER_CLOSE_IDLE_ROOTS, "false").equals( + "true"); + } + private Logger getLogger() { return Logger.getLogger(getClass().getName()); } diff --git a/server/src/com/vaadin/terminal/gwt/server/Constants.java b/server/src/com/vaadin/terminal/gwt/server/Constants.java index a04288055e..adf26bf7f7 100644 --- a/server/src/com/vaadin/terminal/gwt/server/Constants.java +++ b/server/src/com/vaadin/terminal/gwt/server/Constants.java @@ -65,6 +65,7 @@ public interface Constants { static final String SERVLET_PARAMETER_DISABLE_XSRF_PROTECTION = "disable-xsrf-protection"; static final String SERVLET_PARAMETER_RESOURCE_CACHE_TIME = "resourceCacheTime"; static final String SERVLET_PARAMETER_HEARTBEAT_RATE = "heartbeatRate"; + static final String SERVLET_PARAMETER_CLOSE_IDLE_ROOTS = "closeIdleRoots"; // Configurable parameter names static final String PARAMETER_VAADIN_RESOURCES = "Resources"; @@ -95,5 +96,4 @@ public interface Constants { static final String PORTAL_PARAMETER_VAADIN_WIDGETSET = "vaadin.widgetset"; static final String PORTAL_PARAMETER_VAADIN_RESOURCE_PATH = "vaadin.resources.path"; static final String PORTAL_PARAMETER_VAADIN_THEME = "vaadin.theme"; - } diff --git a/server/src/com/vaadin/terminal/gwt/server/PortletApplicationContext2.java b/server/src/com/vaadin/terminal/gwt/server/PortletApplicationContext2.java index eba7d6e3a3..a7ee5be96d 100644 --- a/server/src/com/vaadin/terminal/gwt/server/PortletApplicationContext2.java +++ b/server/src/com/vaadin/terminal/gwt/server/PortletApplicationContext2.java @@ -404,6 +404,11 @@ public class PortletApplicationContext2 extends AbstractWebApplicationContext { } } + @Override + public int getMaxInactiveInterval() { + return getPortletSession().getMaxInactiveInterval(); + } + private Logger getLogger() { return Logger.getLogger(PortletApplicationContext2.class.getName()); } diff --git a/server/src/com/vaadin/terminal/gwt/server/WebApplicationContext.java b/server/src/com/vaadin/terminal/gwt/server/WebApplicationContext.java index 4cc0ed188d..bfcc0c1038 100644 --- a/server/src/com/vaadin/terminal/gwt/server/WebApplicationContext.java +++ b/server/src/com/vaadin/terminal/gwt/server/WebApplicationContext.java @@ -187,4 +187,8 @@ public class WebApplicationContext extends AbstractWebApplicationContext { return mgr; } + @Override + public int getMaxInactiveInterval() { + return getHttpSession().getMaxInactiveInterval(); + } } diff --git a/server/src/com/vaadin/ui/Root.java b/server/src/com/vaadin/ui/Root.java index 14abf0f24f..98c200cd34 100644 --- a/server/src/com/vaadin/ui/Root.java +++ b/server/src/com/vaadin/ui/Root.java @@ -488,6 +488,8 @@ public abstract class Root extends AbstractComponentContainer implements */ private long lastHeartbeat = System.currentTimeMillis(); + private long lastUidlRequest = System.currentTimeMillis(); + /** * Creates a new empty root without a caption. This root will have a * {@link VerticalLayout} with margins enabled as its content. @@ -1319,26 +1321,43 @@ public abstract class Root extends AbstractComponentContainer implements } /** - * Returns the timestamp (millisecond since the epoch) of the last received + * Returns the timestamp (milliseconds since the epoch) of the last received * heartbeat for this Root. * * @see #heartbeat() * @see Application#closeInactiveRoots() * - * @return The time + * @return The time the last heartbeat request occurred. */ - public long getLastHeartbeat() { + public long getLastHeartbeatTime() { return lastHeartbeat; } /** - * Updates the heartbeat timestamp of this Root to the current time. Called - * by the framework whenever the application receives a valid heartbeat or + * Returns the timestamp (milliseconds since the epoch) of the last received * UIDL request for this Root. * - * @see java.lang.System#currentTimeMillis() + * @return + */ + public long getLastUidlRequestTime() { + return lastUidlRequest; + } + + /** + * Sets the last heartbeat request timestamp for this Root. Called by the + * framework whenever the application receives a valid heartbeat request for + * this Root. + */ + public void setLastHeartbeatTime(long lastHeartbeat) { + this.lastHeartbeat = lastHeartbeat; + } + + /** + * Sets the last UIDL request timestamp for this Root. Called by the + * framework whenever the application receives a valid UIDL request for this + * Root. */ - public void heartbeat() { - this.lastHeartbeat = System.currentTimeMillis(); + public void setLastUidlRequestTime(long lastUidlRequest) { + this.lastUidlRequest = lastUidlRequest; } } -- 2.39.5