Stateful vs. stateless interceptors
ovidiu.feodorov Oct 25, 2005 11:38 PMThe current Messaging implementation uses client-side dynamic proxies to implement delegates (ConnectionDelegate, SessionDelegate, aso). Their invocation handlers contain arrays of AOP interceptors. The dynamic proxies are constructed on the server and shipped on the client each time a new top-level JMS API element is created. They may be either serialized if the client that requests their creation is remote, or just passed by reference, if the client and the server are co-located.
The latest decision relative to how to handle the state for client-side delegates was to use stateful interceptors. Relevant state is stored by corresponding interceptors. Case in point: the TransactedInterceptor stores whether a session is transacted or not, by setting its "transacted" protected member. Obviously, this implies with necessity that each session delegate must have its unique TransactedInterceptor instance. If not, creating more than one session per VM would lead to corrupted state. This is what unfortunately happens now.
Run SessionTest.testCreateTwoSessions() in a co-located configuration to get proof. Another test that exhibits the same behavior is SessionTest.testCloseAndCreateSession(). Both tests currently fail (in a co-located configuration). For a non co-located configuration, the tests will pass, because (I assume) the PER_CLASS semantics is not maintained over serialization, so each delegate ends with its own interceptor instance (to be confirmed).
I see at least two ways to fix the problem.
1. Keep the interceptors stateful, but declare them PER_INSTANCE instead of PER_CLASS in jms-aop.xml. I think this solution may be made to work, but there are a few complications that must be resolved first: declaring an interceptor PER_INSTANCE will result in the creation of a PerInstanceInterceptor which delegates to our interceptor only if the invocation has a non-null targetObject (otherwise the invocation is considered static). So when JMSInvocationHandler creates the invocation, we need some sort of targetObject, which implements Advised and it's able to return an InstanceAdvisor, aso. I haven't explored this all the way to the end, but it already looks complicated to me. It also implies a lot of new object instances creations per request.
2. Make the interceptors stateless. Delegate state management to JMSInvocationHandler instances. Arguments in favor of this decision:
* There is a natural one-to-one relationship between delegate instances and their invocation handlers. We don't need to write/configure anything to enforce this.
* We can easily type invocation handlers (we are already using a JMSConsumerInvocationHandler). This will make the whole architecture easier to understand.
* We can easily add typed getters/setters to take the pain out of the interaction with the detyped metadata
So, instead keeping the "transacted" flag inside the TransactedInterceptor, we declare a SessionInvocationHandler with a getTransacted()/setTransacted(boolean).
The interceptor code will look similar to:
public class TransactionInterceptor implements Interceptor, Serializable { // Attributes ---------------------------------------------------- // nothing .... // Interceptor implementation ------------------------------------ ... public Object invoke(Invocation invocation) throws Throwable { if (!(invocation instanceof JMSMethodInvocation)) { return invocation.invokeNext(); } JMSMethodInvocation mi = (JMSMethodInvocation)invocation; String methodName = mi.getMethod().getName(); if ("setTransacted".equals(methodName)) { boolean t = ((Boolean)mi.getArguments()[0]).booleanValue(); ((SessionInvocationHandler)mi.getHandler()).setTransacted(t); return null; } ... } ... }