Trouble with converting an AspectJ program

[prilmeie]

Hello,

I am writing my masther's thesis about aop and I am about to write a definitions and overview chapter. In that chapter I am giving an overview over existing AOP systems, and JBoss AOP is surely a candidate I want to be in that chapter. My plans were to create a common example and to show how and why the implementation of the example differ.

For that reason I have a simple example, a stack class:

public class Stack
{
 private class StackObject
 {
 private Object content;
 private StackObject next = null;

 public StackObject ( Object o, StackObject s )
 {
 this.content = o;
 this.next = s;
 }

 public StackObject getNext ()
 {
 return this.next;
 }

 public Object getContent ()
 {
 return this.content;
 }
 }

 private StackObject top = null;
 private int height = 0;

 public Stack () {}

 public synchronized Object pop ()
 {
 if ( this.top == null )
 {
 // contract violation - a real program would throw an exception
 return null;
 }

 -- this.height;

 StackObject temp = this.top;
 this.top = temp.getNext ();

 return temp.getContent ();
 }

 public synchronized void push ( Object o )
 {
 this.top = new StackObject ( o, this.top );

 ++ this.height;
 }

 public synchronized int size ()
 {
 return this.height;
 }
}

Now I have "identified" two aspects:
* Size
* Synchronization

Contract Enforcement would be a candidate, too, but I didn't want to overcomplicate the example. So my AspectJ implementation is (untested, but compiles):

Stripped down stack class:

public class Stack
{
 private class StackObject
 {
 private Object content;
 private StackObject next = null;

 public StackObject ( Object o, StackObject s )
 {
 this.content = o;
 this.next = s;
 }

 public StackObject getNext ()
 {
 return this.next;
 }

 public Object getContent ()
 {
 return this.content;
 }
 }

 private StackObject top = null;

 public Stack () {}

 public Object pop ()
 {
 if ( this.top == null )
 {
 // contract violation - a real program would throw an exception
 return null;
 }

 Object temp = this.top.getContent ();
 this.top = this.top.getNext ();

 return temp;
 }

 public void push ( Object o )
 {
 this.top = new StackObject ( o, this.top );
 }
}

The size aspect:

public aspect StackHeight
{
 private int Stack.height = 0;

 public int Stack.size ()
 {
 return this.height;
 }

 after ( Stack s ) returning: target ( s ) &&
 call( public void Stack.push(..) )
 {
 ++ s.height;
 }

 after ( Stack s ) returning: target ( s ) &&
 call( public * Stack.pop(..) )
 {
 if ( s.height > 0 )
 {
 -- s.height;
 }
 }
}

And the synchronization aspect:

public aspect StackSynchronization
{
 // 1:
 private boolean Stack.semaphore = false;

 // 2:
 before ( Stack s ): target ( s ) &&
 call( public * Stack.*(..) ) && within( Stack )
 {
 synchronized ( s )
 {
 if ( s.semaphore == true )
 {
 try {
 s.wait ();
 } catch(InterruptedException e) {}
 }

 s.semaphore = true;
 }
 }

 // 3:
 after ( Stack s ): target ( s ) &&
 call( public * Stack.*(..) ) && within( Stack )
 {
 s.semaphore = false;
 s.notify ();
 }
}

My problem here is that I use static introductions and I wasn't able to figure out how this is done in JBoss AOP. I know how changes in the hierarchy and the type system are done, but not how I can change the static structure of a class with JBoss AOP. - Is this even impossible with JBoss AOP?

Any help appriciated.

Many thanks in advance,
Franz

[kabirkhan]

I'm not familiar with AspectJ, but I think the examples you show (if I have understood them correctly) should be possible with JBossAOP. I'm just thinking loud, but with a combination of mixin classes and per instance aspects I reckon you could acheive what you're after. They are documented on the Wiki.

Also, in the wiki, and in the aspects project (where there are som tests) you can find documentation/examples of applying read-write locking via aspects. Synchronization should be similar...

[prilmeie]

Mixins are definitely a good start, but nevertheless it's very hard, because I have to split the aspect into the static (mixin) and the dynamic part (interception). My feelings about this are that this is not in the spirit of AOP.

But let's get to the facts. As far as I understand the WIKI aspect example, I have to it this way:

Write an interface

Write an implementation of the interface (mixin)

Write two interceptor classes (aspects)

Here we go; First of all the interface:

public interface IStackHeight
{
 public int size ();
}

Next comes the mixin. Here I am doing a cheap trick. I need the implementation of the size method in order to satisfy the compiler, but later on, I will use an around advice so this gets never executed; Dull dummy code.

public class StackHeight implements IStackHeight
{
 public int size ()
 {
 return 0;
 }
}

The StackHeightAspect class. The implementation is similar to the wiki:

import org.jboss.aop.joinpoint.*;

public class StackHeightAspect
{
 private int height;

 public Object pushInvocation ( MethodInvocation invocation ) throws Throwable
 {
 ++ this.height;
 return invocation.invokeNext();
 }

 public Object popInvocation ( MethodInvocation invocation ) throws Throwable
 {
 if ( this.height > 0 )
 {
 -- this.height;
 }

 return invocation.invokeNext();
 }

 public int sizeInvocation ( MethodInvocation invocation ) throws Throwable
 {
 return this.height;
 }
}

Everyone should see the cheap trick I am doing here, the actual implementation of the size method is here, not in the mixin! Apart from that I am not sure if the chosen return type, int, is correct. But how would have been the implementation if I am to return an Object?

Now the last part, the synchronization aspect, should be straight forward, but I have my problems here, too:

import org.jboss.aop.joinpoint.*;

public class StackSynchronizationAspect
{
 private boolean semaphore = false;

 public Object synchronize ( MethodInvocation invocation ) throws Throwable
 {
 // synchronized ( invocation.getInstance () )
 synchronized ( this )
 {
 if ( this.semaphore == true )
 {
 try {
 this.wait ();
 } catch(InterruptedException e) {}

 this.semaphore = true;
 }
 }

 Object result = invocation.invokeNext();

 // synchronized ( invocation.getInstance () )
 synchronized ( this )
 {
 this.semaphore = false;

 this.notify ();
 }

 return result;
 }
}

I want to express in the synchronized statements that the synchronization is about the instance of the message receiver. That's what invocation.getInstance should indicate. How is this expressed? Surely somehow with context information, but the docs are very sparse about that.

Now let's put the pieces together:

<?xml version="1.0" encoding="UTF-8"?>
<aop>
 <aspect class="StackHeightAspect" scope="PER_INSTANCE"/>
 <aspect class="StackSynchronizationAspect" scope="PER_INSTANCE"/>

 <introduction class="Stack">
 <mixin>
 <interfaces>
 IStackHeight
 </interfaces>
 <class>StackHeight</class>
 </mixin>
 </introduction>

 <pointcut name="pushExecution" expr="execution(void Stack->push(Object))" />
 <pointcut name="popExecution" expr="execution(Object Stack->pop())" />
 <pointcut name="sizeExecution" expr="execution(int Stack->size())" />

 <bind pointcut="popExecution">
 <advice name="popInvocation" aspect="StackHeightAspect"/>
 </bind>

 <bind pointcut="pushExecution OR popExecution OR sizeExecution">
 <advice name="synchronize" aspect="StackHeightAspect"/>
 </bind>

 <bind pointcut="pushExecution">
 <advice name="pushInvocation" aspect="StackHeightAspect"/>
 </bind>

 <bind pointcut="sizeExecution">
 <advice name="sizeInvocation" aspect="StackHeightAspect"/>
 </bind>
</aop>

This is my test driver (synchronization is not tested here):

public class Driver
{
 public static void main ( String args [] ) throws Exception
 {
 Stack s = new Stack ();

 Driver.printHeight ( s );

 s.push ( new Integer ( 5 ) );
 Driver.printHeight ( s );

 s.push ( new Integer ( 5 ) );
 s.pop ();
 Driver.printHeight ( s );
 }

 private static void printHeight ( Stack s ) throws Exception
 {
 try
 {
 System.out.println ( "Stack size: " + ( (IStackHeight) s ).size () );
 }
 catch ( Exception e )
 {
 System.err.println ( "Introduction failed: " + e );
 }
 }
}

These are the results of my test run:

prilmeie@atlab1 /cygdrive/f/Inetpub/cvsroot/da-aop/source/stack/jboss-aop
$ rm *.class; ant
Buildfile: build.xml

prepare:

compile:
 [javac] Compiling 6 source files to F:\Inetpub\cvsroot\da-aop\source\stack\jboss-aop
 [aopc] [deploying] file:/F:/Inetpub/cvsroot/da-aop/source/stack/jboss-aop/jboss-aop.xml
 [aopc] [deploy] <aspect> file:/F:/Inetpub/cvsroot/da-aop/source/stack/jboss-aop/jboss-aop.xml
 [aopc] [deploy] <aspect> file:/F:/Inetpub/cvsroot/da-aop/source/stack/jboss-aop/jboss-aop.xml
 [aopc] [deploy] <introduction> file:/F:/Inetpub/cvsroot/da-aop/source/stack/jboss-aop/jboss-aop.xml
 [aopc] [deploy] <pointcut> file:/F:/Inetpub/cvsroot/da-aop/source/stack/jboss-aop/jboss-aop.xml
 [aopc] [deploy] <pointcut> file:/F:/Inetpub/cvsroot/da-aop/source/stack/jboss-aop/jboss-aop.xml
 [aopc] [deploy] <pointcut> file:/F:/Inetpub/cvsroot/da-aop/source/stack/jboss-aop/jboss-aop.xml
 [aopc] [deploy] <bind> file:/F:/Inetpub/cvsroot/da-aop/source/stack/jboss-aop/jboss-aop.xml
 [aopc] [deploy] <bind> file:/F:/Inetpub/cvsroot/da-aop/source/stack/jboss-aop/jboss-aop.xml
 [aopc] [deploy] <bind> file:/F:/Inetpub/cvsroot/da-aop/source/stack/jboss-aop/jboss-aop.xml
 [aopc] [deploy] <bind> file:/F:/Inetpub/cvsroot/da-aop/source/stack/jboss-aop/jboss-aop.xml
 [aopc] [trying to transform] Driver
 [aopc] [no comp needed] F:\Inetpub\cvsroot\da-aop\source\stack\jboss-aop\Driver.class
 [aopc] [cannot compile] isInterface: IStackHeight
 [aopc] [no comp needed] F:\Inetpub\cvsroot\da-aop\source\stack\jboss-aop\IStackHeight.class
 [aopc] [trying to transform] Stack$StackObject
 [aopc] [no comp needed] F:\Inetpub\cvsroot\da-aop\source\stack\jboss-aop\Stack$StackObject.class
 [aopc] [trying to transform] Stack
 [aopc] [compiled] F:\Inetpub\cvsroot\da-aop\source\stack\jboss-aop\Stack.class
 [aopc] [trying to transform] StackHeight
 [aopc] [no comp needed] F:\Inetpub\cvsroot\da-aop\source\stack\jboss-aop\StackHeight.class
 [aopc] [trying to transform] StackHeightAspect
 [aopc] [no comp needed] F:\Inetpub\cvsroot\da-aop\source\stack\jboss-aop\StackHeightAspect.class
 [aopc] [trying to transform] StackSynchronizationAspect
 [aopc] [no comp needed] F:\Inetpub\cvsroot\da-aop\source\stack\jboss-aop\StackSynchronizationAspect.class



run:
 [java] Introduction failed: java.lang.ClassCastException
 [java] Introduction failed: java.lang.ClassCastException
 [java] Introduction failed: java.lang.ClassCastException



BUILD SUCCESSFUL
Total time: 3 seconds

So the whole work was for nothing? - There must be a simple explanation for this, what went wrong?

Trying to disassemble Stack:

prilmeie@atlab1 /cygdrive/f/Inetpub/cvsroot/da-aop/source/stack/jboss-aop
$ javap -c Stack
Compiled from "Stack.java"
public class Stack extends java.lang.Object implements org.jboss.aop.Advised,IStackHeight{
protected transient org.jboss.aop.ClassInstanceAdvisor _instanceAdvisor;

public Stack();
 Code: [...]

Why does the class cast fail? The whole package can be loaded at http://home.in.tum.de/prilmeie/jboss-aop-stack.tar.gz], please make sure that jboss-common.jar, jboss-aop.jar, javassist.jar, concurrent.jar, trove.jar and qdox.jar are in the same directory as the extracted files are. Otherwise it will not work

Apart from that I am very dissatisfied with the results. I had to use a cheap trick to get the results I wanted. Worse, the StackHeightAspect is now a necessary part of the Stack class, otherwise I never can use the size method. I even have to use the silly class cast whenever I want to use that specific method. A maintenance nightmare! - There must be a better way to achieve these results (This was my first attempt to use JBoss AOP, used version is beta3)

Are there any other mistakes I have made?

[kabirkhan]

I took a look at your example:

1) Initially when I ran it the class cast failed. However, I found this to be due to running within Eclipse, with your jboss-aop-stack on the source path. And my personal build.xml is set up to use the Eclipse output classes. For some reason, this was causing Stack to not implement IStackHeight on my setup. I'm not sure why...

2) Next I ran into another ClassCastException occuring inside the interceptors when trying to intercept introduced methods. I've fixed that bug, get the latest from cvs. Hard to say if 1) or 2) caused your problems, but give it a go.

3) Regarding your separation of the the static/dynamic stuff I deleted the StackHeight class.

StackHeightAspect now contains "everything" - no cheap tricks ;-)

public class StackHeightAspect implements IStackHeight
{
 private int height;

 public Object pushInvocation ( MethodInvocation invocation ) throws Throwable
 {
 ++ this.height;
 return invocation.invokeNext();
 }

 public Object popInvocation ( MethodInvocation invocation ) throws Throwable
 {
 if ( this.height > 0 )
 {
 -- this.height;
 }

 return invocation.invokeNext();
 }

 //As per documentation advices must return Object
 public Object sizeInvocation ( MethodInvocation invocation ) throws Throwable
 {
 return new Integer(this.height);
 }
}

jboss-aop.xml:

<aop>
 <aspect class="StackHeightAspect" scope="PER_INSTANCE"/>
 <aspect class="StackSynchronizationAspect" scope="PER_INSTANCE"/>

 <introduction class="Stack">
 <mixin>
 <interfaces>
 IStackHeight
 </interfaces>
 <class>StackHeightAspect</class>
 </mixin>
 </introduction>

 <!-- Note that the fully qualified name of the class must be used -->
 <pointcut name="pushExecution" expr="execution(void Stack->push(java.lang.Object))" />
 <pointcut name="popExecution" expr="execution(java.lang.Object Stack->pop())" />
 <pointcut name="sizeExecution" expr="execution(int Stack->size())" />

 <bind pointcut="popExecution">
 <advice name="popInvocation" aspect="StackHeightAspect"/>
 </bind>

 <!-- This was bound to StackHeightAspect, but the advice is in StackHeightAspect -->
 <bind pointcut="pushExecution OR popExecution OR sizeExecution">
 <advice name="synchronize" aspect="StackSynchronizationAspect"/>
 </bind>

 <bind pointcut="pushExecution">
 <advice name="pushInvocation" aspect="StackHeightAspect"/>
 </bind>

 <bind pointcut="sizeExecution">
 <advice name="sizeInvocation" aspect="StackHeightAspect"/>
 </bind>

</aop>

4) Synchronization aspect. You use invocation.getTargetObject():

public class StackSynchronizationAspect
{
 private boolean semaphore = false;

 public Object synchronize ( MethodInvocation invocation ) throws Throwable
 {
 System.out.println("synch aspect");
 // synchronized ( invocation.getInstance () )
 Object obj = invocation.getTargetObject();
 synchronized ( obj )
 {
 if ( this.semaphore == true )
 {
 try {
 obj.wait ();
 } catch(InterruptedException e) {}

 this.semaphore = true;
 }
 }

 Object result = invocation.invokeNext();

 // synchronized ( invocation.getInstance () )
 synchronized ( obj )
 {
 this.semaphore = false;

 obj.notify ();
 }

 return result;
 }
}

Cheers,

Kab

[kabirkhan]

Ooooops!

public class StackHeightAspect implements IStackHeight
{
 private int height;

 public int size ()
 {
 return height;
 }

 public Object pushInvocation ( MethodInvocation invocation ) throws Throwable
 {
 ++ this.height;
 return invocation.invokeNext();
 }

 public Object popInvocation ( MethodInvocation invocation ) throws Throwable
 {
 if ( this.height > 0 )
 {
 -- this.height;
 }

 return invocation.invokeNext();
 }

 public Object sizeInvocation ( MethodInvocation invocation ) throws Throwable
 {
 return new Integer(this.size());
 }
}

[prilmeie]

Many, many thanks, I would have been lost without you! You saved me at least one week reading (and trying to understand) the JBoss AOP source code.