From 81922cb88806b211fb820368beed5a0c283c5b3c Mon Sep 17 00:00:00 2001 From: Keith Donald Date: Mon, 31 Mar 2008 19:12:59 +0000 Subject: [PATCH] backport concurrent timeoutSeconds polish in conversation management system --- spring-webflow/.classpath | 2 +- spring-webflow/ivy.xml | 2 +- .../webflow/conversation/Conversation.java | 3 +- .../conversation/ConversationException.java | 4 +- .../ConversationLockException.java | 27 ++++++ .../impl/ContainedConversation.java | 20 +---- .../impl/ConversationContainer.java | 8 +- .../conversation/impl/ConversationLock.java | 9 +- .../impl/ConversationLockFactory.java | 44 ++++++--- ...JdkBackportConcurrentConversationLock.java | 57 ++++++++++++ .../impl/JdkConcurrentConversationLock.java | 18 ++-- .../conversation/impl/LockException.java | 9 -- .../impl/NoOpConversationLock.java | 3 +- .../SessionBindingConversationManager.java | 50 ++++++++--- .../impl/SimpleConversationId.java | 3 +- .../impl/UtilConcurrentConversationLock.java | 90 ------------------- 16 files changed, 183 insertions(+), 166 deletions(-) create mode 100644 spring-webflow/src/main/java/org/springframework/webflow/conversation/ConversationLockException.java create mode 100644 spring-webflow/src/main/java/org/springframework/webflow/conversation/impl/JdkBackportConcurrentConversationLock.java delete mode 100644 spring-webflow/src/main/java/org/springframework/webflow/conversation/impl/LockException.java delete mode 100644 spring-webflow/src/main/java/org/springframework/webflow/conversation/impl/UtilConcurrentConversationLock.java diff --git a/spring-webflow/.classpath b/spring-webflow/.classpath index 53fd089a..c8f80653 100644 --- a/spring-webflow/.classpath +++ b/spring-webflow/.classpath @@ -6,7 +6,6 @@ - @@ -39,5 +38,6 @@ + diff --git a/spring-webflow/ivy.xml b/spring-webflow/ivy.xml index 7b3f7766..71aeef9e 100644 --- a/spring-webflow/ivy.xml +++ b/spring-webflow/ivy.xml @@ -47,7 +47,7 @@ - + diff --git a/spring-webflow/src/main/java/org/springframework/webflow/conversation/Conversation.java b/spring-webflow/src/main/java/org/springframework/webflow/conversation/Conversation.java index 5a795cfe..bc3c04d0 100644 --- a/spring-webflow/src/main/java/org/springframework/webflow/conversation/Conversation.java +++ b/spring-webflow/src/main/java/org/springframework/webflow/conversation/Conversation.java @@ -57,8 +57,9 @@ public interface Conversation { /** * Lock this conversation. May block until the lock is available, if someone else has acquired the lock. + * @throws ConversationLockException if the lock could not be acquired */ - public void lock(); + public void lock() throws ConversationLockException; /** * Returns the conversation attribute with the specified name. You need to aquire the lock on this conversation diff --git a/spring-webflow/src/main/java/org/springframework/webflow/conversation/ConversationException.java b/spring-webflow/src/main/java/org/springframework/webflow/conversation/ConversationException.java index 7aebc7f6..6bece0e2 100644 --- a/spring-webflow/src/main/java/org/springframework/webflow/conversation/ConversationException.java +++ b/spring-webflow/src/main/java/org/springframework/webflow/conversation/ConversationException.java @@ -15,14 +15,12 @@ */ package org.springframework.webflow.conversation; -import org.springframework.webflow.core.FlowException; - /** * The root of the conversation service exception hierarchy. * * @author Keith Donald */ -public class ConversationException extends FlowException { +public abstract class ConversationException extends RuntimeException { /** * Creates a conversation service exception. diff --git a/spring-webflow/src/main/java/org/springframework/webflow/conversation/ConversationLockException.java b/spring-webflow/src/main/java/org/springframework/webflow/conversation/ConversationLockException.java new file mode 100644 index 00000000..d46b5030 --- /dev/null +++ b/spring-webflow/src/main/java/org/springframework/webflow/conversation/ConversationLockException.java @@ -0,0 +1,27 @@ +package org.springframework.webflow.conversation; + + +/** + * An exception occurred within the conversation locking system. + * + * @author Keith Donald + */ +public abstract class ConversationLockException extends ConversationException { + + /** + * Creates a new lock exception. + * @param msg the Exception message + */ + public ConversationLockException(String msg) { + super(msg); + } + + /** + * Creates a new lock exception. + * @param msg the Exception message + * @param cause the root cause of this Exception + */ + public ConversationLockException(String msg, Throwable cause) { + super(msg, cause); + } +} diff --git a/spring-webflow/src/main/java/org/springframework/webflow/conversation/impl/ContainedConversation.java b/spring-webflow/src/main/java/org/springframework/webflow/conversation/impl/ContainedConversation.java index 7e1fda3c..cf43d676 100644 --- a/spring-webflow/src/main/java/org/springframework/webflow/conversation/impl/ContainedConversation.java +++ b/spring-webflow/src/main/java/org/springframework/webflow/conversation/impl/ContainedConversation.java @@ -15,9 +15,6 @@ */ package org.springframework.webflow.conversation.impl; -import java.io.IOException; -import java.io.ObjectInputStream; -import java.io.ObjectOutputStream; import java.io.Serializable; import java.util.HashMap; import java.util.Map; @@ -44,7 +41,7 @@ class ContainedConversation implements Conversation, Serializable { private ConversationId id; - private transient ConversationLock lock; + private ConversationLock lock; private Map attributes; @@ -52,11 +49,12 @@ class ContainedConversation implements Conversation, Serializable { * Create a new contained conversation. * @param container the container containing the conversation * @param id the unique id assigned to the conversation + * @param lock the conversation lock */ - public ContainedConversation(ConversationContainer container, ConversationId id) { + public ContainedConversation(ConversationContainer container, ConversationId id, ConversationLock lock) { this.container = container; this.id = id; - this.lock = ConversationLockFactory.createLock(); + this.lock = lock; this.attributes = new HashMap(); } @@ -129,14 +127,4 @@ class ContainedConversation implements Conversation, Serializable { return id.hashCode(); } - // custom serialization - - private void writeObject(ObjectOutputStream out) throws IOException { - out.defaultWriteObject(); - } - - private void readObject(ObjectInputStream in) throws IOException, ClassNotFoundException { - in.defaultReadObject(); - lock = ConversationLockFactory.createLock(); - } } \ No newline at end of file diff --git a/spring-webflow/src/main/java/org/springframework/webflow/conversation/impl/ConversationContainer.java b/spring-webflow/src/main/java/org/springframework/webflow/conversation/impl/ConversationContainer.java index 8f25f81f..25f03953 100644 --- a/spring-webflow/src/main/java/org/springframework/webflow/conversation/impl/ConversationContainer.java +++ b/spring-webflow/src/main/java/org/springframework/webflow/conversation/impl/ConversationContainer.java @@ -82,11 +82,13 @@ class ConversationContainer implements Serializable { /** * Create a new conversation based on given parameters and add it to the container. - * @param parameters descriptive parameters + * @param parameters descriptive conversation parameters + * @param lockFactory the lock factory to use to create the conversation lock * @return the created conversation */ - public synchronized Conversation createConversation(ConversationParameters parameters) { - ContainedConversation conversation = new ContainedConversation(this, nextId()); + public synchronized Conversation createConversation(ConversationParameters parameters, + ConversationLockFactory lockFactory) { + ContainedConversation conversation = new ContainedConversation(this, nextId(), lockFactory.createLock()); conversations.add(conversation); if (maxExceeded()) { // end oldest conversation diff --git a/spring-webflow/src/main/java/org/springframework/webflow/conversation/impl/ConversationLock.java b/spring-webflow/src/main/java/org/springframework/webflow/conversation/impl/ConversationLock.java index f941304d..6f5b0946 100644 --- a/spring-webflow/src/main/java/org/springframework/webflow/conversation/impl/ConversationLock.java +++ b/spring-webflow/src/main/java/org/springframework/webflow/conversation/impl/ConversationLock.java @@ -15,17 +15,22 @@ */ package org.springframework.webflow.conversation.impl; +import java.io.Serializable; + +import org.springframework.webflow.conversation.ConversationLockException; + /** * A normalized interface for conversation locks, used to obtain exclusive access to a conversation. * * @author Keith Donald */ -public interface ConversationLock { +public interface ConversationLock extends Serializable { /** * Acquire the conversation lock. + * @throws ConversationLockException if an exception is thrown attempting to acquire this lock */ - public void lock(); + public void lock() throws ConversationLockException; /** * Release the conversation lock. diff --git a/spring-webflow/src/main/java/org/springframework/webflow/conversation/impl/ConversationLockFactory.java b/spring-webflow/src/main/java/org/springframework/webflow/conversation/impl/ConversationLockFactory.java index 65f9cb97..1bd0a3be 100644 --- a/spring-webflow/src/main/java/org/springframework/webflow/conversation/impl/ConversationLockFactory.java +++ b/spring-webflow/src/main/java/org/springframework/webflow/conversation/impl/ConversationLockFactory.java @@ -25,34 +25,52 @@ import org.springframework.core.JdkVersion; * @author Keith Donald * @author Rob Harrop */ -public class ConversationLockFactory { +class ConversationLockFactory { private static final Log logger = LogFactory.getLog(ConversationLockFactory.class); - private static boolean utilConcurrentPresent; + private static boolean backportConcurrentPresent; static { try { - Class.forName("EDU.oswego.cs.dl.util.concurrent.ReentrantLock"); - utilConcurrentPresent = true; + Class.forName("edu.emory.mathcs.backport.java.util.concurrent.locks.ReentrantLock"); + backportConcurrentPresent = true; } catch (ClassNotFoundException ex) { - utilConcurrentPresent = false; + backportConcurrentPresent = false; } } + private int timeoutSeconds = 30; + /** - * When running on Java 1.5+, returns a jdk5 concurrent lock. When running on older JDKs with the 'util.concurrent' - * package available, returns a util concurrent lock. In all other cases a "no-op" lock is returned. + * Returns the period of time that can elapse before a lock attempt times out for locks created by this factory. */ - public static ConversationLock createLock() { + public int getTimeoutSeconds() { + return timeoutSeconds; + } + + /** + * Sets the period of time that can elapse before a lock attempt times out for locks created by this factory. + * @param timeoutSeconds the timeout period in seconds + */ + public void setTimeoutSeconds(int timeoutSeconds) { + this.timeoutSeconds = timeoutSeconds; + } + + /** + * When running on Java 1.5+, returns a jdk5 concurrent lock. When running on older JDKs with the + * 'backport-util-concurrent' package available, returns a backport concurrent lock. In all other cases a "no-op" + * lock is returned. + */ + public ConversationLock createLock() { if (JdkVersion.getMajorJavaVersion() >= JdkVersion.JAVA_15) { - return new JdkConcurrentConversationLock(); - } else if (utilConcurrentPresent) { - return new UtilConcurrentConversationLock(); + return new JdkConcurrentConversationLock(timeoutSeconds); + } else if (backportConcurrentPresent) { + return new JdkBackportConcurrentConversationLock(timeoutSeconds); } else { logger.warn("Unable to enable conversation locking. Switch to Java 5 or above, " - + "or put the 'util.concurrent' package on the classpath " - + "to enable locking in your environment."); + + "or put the 'backport-util-concurrent' package on the classpath " + + "to enable locking in your Java 1.4 environment."); return NoOpConversationLock.INSTANCE; } } diff --git a/spring-webflow/src/main/java/org/springframework/webflow/conversation/impl/JdkBackportConcurrentConversationLock.java b/spring-webflow/src/main/java/org/springframework/webflow/conversation/impl/JdkBackportConcurrentConversationLock.java new file mode 100644 index 00000000..eedd3672 --- /dev/null +++ b/spring-webflow/src/main/java/org/springframework/webflow/conversation/impl/JdkBackportConcurrentConversationLock.java @@ -0,0 +1,57 @@ +/* + * Copyright 2004-2007 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.springframework.webflow.conversation.impl; + +import org.springframework.webflow.conversation.ConversationLockException; + +import edu.emory.mathcs.backport.java.util.concurrent.TimeUnit; +import edu.emory.mathcs.backport.java.util.concurrent.locks.Lock; +import edu.emory.mathcs.backport.java.util.concurrent.locks.ReentrantLock; + +/** + * A conversation lock that relies on backport-concurrent. For use on JDK 1.4 + * + * @author Keith Donald + * @author Rob Harrop + */ +class JdkBackportConcurrentConversationLock implements ConversationLock { + + private Lock lock = new ReentrantLock(); + + private int timeoutSeconds; + + public JdkBackportConcurrentConversationLock(int timeoutSeconds) { + this.timeoutSeconds = timeoutSeconds; + } + + public void lock() throws ConversationLockException { + try { + boolean acquired = lock.tryLock(timeoutSeconds, TimeUnit.SECONDS); + if (!acquired) { + throw new LockTimeoutException(timeoutSeconds); + } + } catch (InterruptedException e) { + throw new LockInterruptedException(e); + } + } + + /** + * Releases the lock. + */ + public void unlock() { + lock.unlock(); + } +} \ No newline at end of file diff --git a/spring-webflow/src/main/java/org/springframework/webflow/conversation/impl/JdkConcurrentConversationLock.java b/spring-webflow/src/main/java/org/springframework/webflow/conversation/impl/JdkConcurrentConversationLock.java index ae6d7991..cbaaf615 100644 --- a/spring-webflow/src/main/java/org/springframework/webflow/conversation/impl/JdkConcurrentConversationLock.java +++ b/spring-webflow/src/main/java/org/springframework/webflow/conversation/impl/JdkConcurrentConversationLock.java @@ -15,38 +15,36 @@ */ package org.springframework.webflow.conversation.impl; -import java.io.Serializable; import java.util.concurrent.TimeUnit; import java.util.concurrent.locks.Lock; import java.util.concurrent.locks.ReentrantLock; +import org.springframework.webflow.conversation.ConversationLockException; + /** * A conversation lock that relies on a {@link ReentrantLock} within Java 5's util.concurrent.locks * package. * * @author Keith Donald */ -class JdkConcurrentConversationLock implements ConversationLock, Serializable { +class JdkConcurrentConversationLock implements ConversationLock { - /** - * The lock. - */ private Lock lock = new ReentrantLock(); - private int timeoutSeconds = 30; + private int timeoutSeconds; - public void setTimeoutSeconds(int timeoutSeconds) { + public JdkConcurrentConversationLock(int timeoutSeconds) { this.timeoutSeconds = timeoutSeconds; } - public void lock() { + public void lock() throws ConversationLockException { try { boolean acquired = lock.tryLock(timeoutSeconds, TimeUnit.SECONDS); if (!acquired) { - throw new LockException("Unable to acquire conversation lock after " + timeoutSeconds + " seconds"); + throw new LockTimeoutException(timeoutSeconds); } } catch (InterruptedException e) { - throw new IllegalStateException("Unable to acquire conversation lock - thread interrupted", e); + throw new LockInterruptedException(e); } } diff --git a/spring-webflow/src/main/java/org/springframework/webflow/conversation/impl/LockException.java b/spring-webflow/src/main/java/org/springframework/webflow/conversation/impl/LockException.java deleted file mode 100644 index 3607fc10..00000000 --- a/spring-webflow/src/main/java/org/springframework/webflow/conversation/impl/LockException.java +++ /dev/null @@ -1,9 +0,0 @@ -package org.springframework.webflow.conversation.impl; - -import org.springframework.webflow.conversation.ConversationException; - -public class LockException extends ConversationException { - public LockException(String message) { - super(message); - } -} diff --git a/spring-webflow/src/main/java/org/springframework/webflow/conversation/impl/NoOpConversationLock.java b/spring-webflow/src/main/java/org/springframework/webflow/conversation/impl/NoOpConversationLock.java index 73159b0a..2ba479d3 100644 --- a/spring-webflow/src/main/java/org/springframework/webflow/conversation/impl/NoOpConversationLock.java +++ b/spring-webflow/src/main/java/org/springframework/webflow/conversation/impl/NoOpConversationLock.java @@ -16,7 +16,6 @@ package org.springframework.webflow.conversation.impl; import java.io.ObjectStreamException; -import java.io.Serializable; /** * A singleton lock that doesn't do anything. For use when conversations don't require or choose not to implement @@ -24,7 +23,7 @@ import java.io.Serializable; * * @author Keith Donald */ -class NoOpConversationLock implements ConversationLock, Serializable { +class NoOpConversationLock implements ConversationLock { /** * The singleton instance. diff --git a/spring-webflow/src/main/java/org/springframework/webflow/conversation/impl/SessionBindingConversationManager.java b/spring-webflow/src/main/java/org/springframework/webflow/conversation/impl/SessionBindingConversationManager.java index 76a5ce04..bb5d1bc3 100644 --- a/spring-webflow/src/main/java/org/springframework/webflow/conversation/impl/SessionBindingConversationManager.java +++ b/spring-webflow/src/main/java/org/springframework/webflow/conversation/impl/SessionBindingConversationManager.java @@ -53,18 +53,9 @@ public class SessionBindingConversationManager implements ConversationManager { private int maxConversations = 5; /** - * Returns the maximum number of allowed concurrent conversations. The default is 5. + * The factory for creating conversation lock objects. */ - public int getMaxConversations() { - return maxConversations; - } - - /** - * Set the maximum number of allowed concurrent conversations. Set to -1 for no limit. The default is 5. - */ - public void setMaxConversations(int maxConversations) { - this.maxConversations = maxConversations; - } + private ConversationLockFactory conversationLockFactory = new ConversationLockFactory(); /** * Returns the key this conversation manager uses to store conversation data in the session. @@ -84,8 +75,41 @@ public class SessionBindingConversationManager implements ConversationManager { this.sessionKey = sessionKey; } + /** + * Returns the maximum number of allowed concurrent conversations. The default is 5. + */ + public int getMaxConversations() { + return maxConversations; + } + + /** + * Set the maximum number of allowed concurrent conversations. Set to -1 for no limit. The default is 5. + */ + public void setMaxConversations(int maxConversations) { + this.maxConversations = maxConversations; + } + + /** + * Returns the time period that can elapse before a timeout occurs on an attempt to acquire a conversation lock. The + * default is 30 seconds. + */ + public int getLockTimeoutSeconds() { + return conversationLockFactory.getTimeoutSeconds(); + } + + /** + * Sets the time period that can elapse before a timeout occurs on an attempt to acquire a conversation lock. The + * default is 30 seconds. + * @param timeoutSeconds the timeout period in seconds + */ + public void setLockTimeoutSeconds(int timeoutSeconds) { + conversationLockFactory.setTimeoutSeconds(timeoutSeconds); + } + + // implementing conversation manager + public Conversation beginConversation(ConversationParameters conversationParameters) throws ConversationException { - return getConversationContainer().createConversation(conversationParameters); + return getConversationContainer().createConversation(conversationParameters, conversationLockFactory); } public Conversation getConversation(ConversationId id) throws ConversationException { @@ -96,7 +120,7 @@ public class SessionBindingConversationManager implements ConversationManager { try { return new SimpleConversationId(Integer.valueOf(encodedId)); } catch (NumberFormatException e) { - throw new ConversationException("Unable to parse string-encoded conversationId + '" + encodedId + "'", e); + throw new BadlyFormattedConversationIdException(encodedId, e); } } diff --git a/spring-webflow/src/main/java/org/springframework/webflow/conversation/impl/SimpleConversationId.java b/spring-webflow/src/main/java/org/springframework/webflow/conversation/impl/SimpleConversationId.java index 9ab466c4..31321b4d 100644 --- a/spring-webflow/src/main/java/org/springframework/webflow/conversation/impl/SimpleConversationId.java +++ b/spring-webflow/src/main/java/org/springframework/webflow/conversation/impl/SimpleConversationId.java @@ -21,8 +21,7 @@ import org.springframework.webflow.conversation.ConversationId; import org.springframework.webflow.conversation.ConversationManager; /** - * An id that uniquely identifies a conversation managed by a {@link ConversationManager}. This key consists of a - * unique string that is typically a GUID. + * An id that uniquely identifies a conversation managed by a {@link ConversationManager}. * * @author Ben Hale */ diff --git a/spring-webflow/src/main/java/org/springframework/webflow/conversation/impl/UtilConcurrentConversationLock.java b/spring-webflow/src/main/java/org/springframework/webflow/conversation/impl/UtilConcurrentConversationLock.java deleted file mode 100644 index d2c73616..00000000 --- a/spring-webflow/src/main/java/org/springframework/webflow/conversation/impl/UtilConcurrentConversationLock.java +++ /dev/null @@ -1,90 +0,0 @@ -/* - * Copyright 2004-2007 the original author or authors. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -package org.springframework.webflow.conversation.impl; - -import org.springframework.core.NestedRuntimeException; - -import EDU.oswego.cs.dl.util.concurrent.ReentrantLock; - -/** - * A conversation lock that relies on a {@link ReentrantLock} within Doug Lea's util.concurrent package. - * For use on JDK 1.3 and 1.4. - * - * @author Keith Donald - * @author Rob Harrop - */ -class UtilConcurrentConversationLock implements ConversationLock { - - /** - * The {@link ReentrantLock} instance. - */ - private final ReentrantLock lock = new ReentrantLock(); - - private int timeoutMills; - - public void setTimeoutSeconds(int timeoutSeconds) { - this.timeoutMills = timeoutSeconds * 1000; - } - - /** - * Acquires the lock. - * @throws SystemInterruptedException if the lock cannot be acquired due to interruption - */ - public void lock() { - try { - boolean acquired = lock.attempt(timeoutMills); - if (!acquired) { - throw new LockException("Unable to acquire conversation lock after " + timeoutMills / 1000 + " seconds"); - } - } catch (InterruptedException e) { - throw new SystemInterruptedException("Unable to acquire conversation lock - thread interrupted", e); - } - } - - /** - * Releases the lock. - */ - public void unlock() { - lock.release(); - } - - /** - * Exception indicating that some {@link Thread} was {@link Thread#interrupt() interrupted} during - * processing and as such processing was halted. - *

- * Only used to wrap the checked {@link InterruptedException java.lang.InterruptedException}. - */ - public static class SystemInterruptedException extends NestedRuntimeException { - - /** - * Creates a new SystemInterruptedException. - * @param msg the Exception message - */ - public SystemInterruptedException(String msg) { - super(msg); - } - - /** - * Creates a new SystemInterruptedException. - * @param msg the Exception message - * @param cause the root cause of this Exception - */ - public SystemInterruptedException(String msg, Throwable cause) { - super(msg, cause); - } - } -} \ No newline at end of file