I am struggling to create a ReentrantReadWriteLock with AspectJ for every single object that is constructed and is a type of Mystructure. Here is my source code.
The aspect class
import org.aspectj.lang.JoinPoint;
import org.aspectj.lang.annotation.After;
import org.aspectj.lang.annotation.Aspect;
import org.aspectj.lang.annotation.Before;
import org.aspectj.lang.annotation.Pointcut;
import java.util.concurrent.locks.Lock;
import java.util.concurrent.locks.ReentrantReadWriteLock;
@Aspect
public class LocksAspect {
private ReentrantReadWriteLock rwLock;
private Lock acquireReadLock;
private Lock acquireWriteLock;
@Before("!within(LocksAspect)&&execution(*.new(..))")
public void LookupBefores() {
rwLock = new ReentrantReadWriteLock();
acquireReadLock = rwLock.readLock();
acquireWriteLock = rwLock.writeLock();
}
@Pointcut("call(void MyStructure.Insert(String))")
public void InsertPointcut() {
}
@Pointcut("call(void MyStructure.Read(int))")
public void ReadPointcut() {
}
@Before("InsertPointcut()")
public void InsertPointcutBefore(JoinPoint pointcut) throws InterruptedException {
acquireWriteLock.lock();
String thrdName = Thread.currentThread().getName();
System.out.println(thrdName + " is entering in critical Section {} ");
Thread.sleep(10000);
}
@After("InsertPointcut()")
public void InsertPointcutAfter(JoinPoint pointcut) {
String thrdName = Thread.currentThread().getName();
System.out.println(thrdName + " received notification and is exiting critical Section {} ");
acquireWriteLock.unlock();
}
@Before("ReadPointcut()")
public void ReadPointcutBefore(JoinPoint pointcut) throws InterruptedException {
acquireReadLock.lock();
String thrdName = Thread.currentThread().getName();
System.out.println(thrdName + " is entering in critical Section {} ");
Thread.sleep(1000);
}
@After("ReadPointcut()")
public void ReadPointcutAfter(JoinPoint pointcut) {
String thrdName = Thread.currentThread().getName();
System.out.println(thrdName + " received notification and is exiting critical Section {} ");
acquireReadLock.unlock();
}
}
The Thread writer class.(The Reader thread class is not important because my problem is different so i omitted it)
public class Writer extends Thread{
private MyStructure myStructure;
public Writer(MyStructure myStructure) {
this.myStructure=myStructure;
}
@Override
public void run() {
this.myStructure.Insert("example");
}
}
My structure class
import java.util.ArrayList;
public class MyStructure {
ArrayList<String> examplelist;
public MyStructure() {
examplelist = new ArrayList<String>();
}
public void Insert(String value) {
examplelist.add(value);
}
public void Read(int pos) {
examplelist.get(pos);
}
}
The main
MyStructure structure = new MyStructure();
MyStructure structure1 = new MyStructure();
new Thread(new Writer(structure), "Thread1").start();
new Thread(new Writer(structure1), "Thread2").start();
The output
Thread2 is entering in critical Section {}
Thread2 received notification and is exiting critical Section {}
Thread1 is entering in critical Section {} //Thread1 will wait for Thread2 to release the lock in critical section which is wrong
Thread1 received notification and is exiting critical Section {}
Now my problem is How I will get a new ReentrantReadWriteLock for each object of Mystructure that created. For example, if we run the above example both Thread1 and Thread2 must be able to access the critical section because they have different references of the object, but this should not have happened. My problem is that the Thread2 will block and wait for Thread1 to finish which is wrong. How can I bypass this problem of construction with Aspect4j?
The key to your problem's solution is that you need one set of locks per MyStructure instance. Your aspect is a singleton, though. So either you need to use another aspect instantiation scheme (which is what I will use in my answer) or do manual bookkeeping within the singleton aspect by keeping a set of locks and add a new element into that set whenever a MyStructure object is created.
In order to better understand my answer, please refer to the AspectJ manual for information about aspect instantiation.
Before we start, a few comments concerning your code and why I have changed it a bit:
- Your
Writer is already a Thread subclass, no need to wrap it into another thread instance. (I know you probably just did it for being able to name the thread, but that could have been achieved by adding a constructor in your class taking the name argument and passing it through to the super class constructor.)
- You should not call a variable of type
JoinPoint pointcut because a joinpoint is not a pointcut AOP-wise.
- I factored out the logging into its own helper method and improved it a bit so we can see more clearly what happens when.
- I decided to replace each pair of before and after advice by an around advice. This is optional, of course, but I prefer to see the control flow in one place in this case. BTW, be careful to change the around advice's return type to
Object and actually return something if you want to target non-void methods. Here it is not necessary because in both cases we have void methods.
- I also decided to inline the pointcuts, which is also optional, but makes the sample code somewhat more concise for demonstration purposes here.
- I added a
Reader class and use it in order to show the difference between reentrant read vs write locks.
- I also took care of making
MyStructure instances nameable and printable in order to identify the target objects more easily in the log.
- I randomised the execution order of reader/writer threads so as to mix them in a more real-world fashion. In order to avoid exceptions polluting the log when reading from a newly created
MyStructure before writing into, I made sure that MyStructure gets a default element right in the constructor. I did not want to catch exceptions here to in order to keep the sample code simple.
- I put the aspect in another package than the application code in order to demonstrate than generally you need to use fully qualified class names when using annotation-style AspectJ (in native syntax imports would be enough).
Now what is the solution? Basically just this, because the changes mentioned above just make the code better or the test program closer to real-life situations:
@Aspect("pertarget(execution(de.scrum_master.app.MyStructure.new(..)))")
public class LocksAspect { // (...)
This creates one aspect instance per MyStructure object. This is also why we can assign the values of readWriteLock, readLock and writeLock directly instead of using a special pointcut + advice pair like in your singleton aspect.
Here is the full, refactored sample code:
Application code + driver application:
package de.scrum_master.app;
import java.util.ArrayList;
import java.util.List;
public class MyStructure {
private String name;
private List<String> myList;
public MyStructure(String name) {
this.name = name;
myList = new ArrayList<String>();
myList.add("dummy element to permit reading");
}
public void insert(String value) {
myList.add(value);
}
public void read(int pos) {
myList.get(pos);
}
@Override
public String toString() {
return "MyStructure[" + name + "]";
}
}
package de.scrum_master.app;
public class Writer extends Thread {
private MyStructure myStructure;
public Writer(MyStructure myStructure) {
this.myStructure = myStructure;
}
@Override
public void run() {
myStructure.insert("example");
}
}
package de.scrum_master.app;
public class Reader extends Thread {
private MyStructure myStructure;
public Reader(MyStructure myStructure) {
this.myStructure = myStructure;
}
@Override
public void run() {
myStructure.read(0);
}
}
package de.scrum_master.app;
import java.util.Arrays;
import java.util.Collections;
import java.util.List;
public class Application {
public static void main(String[] args) {
MyStructure structureA = new MyStructure("One");
MyStructure structureB = new MyStructure("Two");
List<Thread> threads = Arrays.asList(
new Writer(structureA), new Writer(structureB), new Writer(structureA), new Writer(structureB),
new Reader(structureA), new Reader(structureB), new Reader(structureA), new Reader(structureB),
new Reader(structureA), new Reader(structureB), new Reader(structureA), new Reader(structureB)
);
Collections.shuffle(threads);
for (Thread thread : threads)
thread.start();
}
}
Aspect:
package de.scrum_master.aspect;
import java.util.concurrent.locks.Lock;
import java.util.concurrent.locks.ReentrantReadWriteLock;
import org.aspectj.lang.ProceedingJoinPoint;
import org.aspectj.lang.annotation.Around;
import org.aspectj.lang.annotation.Aspect;
import de.scrum_master.app.MyStructure;
@Aspect("pertarget(execution(de.scrum_master.app.MyStructure.new(..)))")
public class LocksAspect {
private static final long startTime = System.currentTimeMillis();
private ReentrantReadWriteLock readWriteLock = new ReentrantReadWriteLock();
private Lock readLock = readWriteLock.readLock();
private Lock writeLock = readWriteLock.writeLock();
@Around("target(myStructure) && execution(void insert(String))")
public void InsertPointcutBefore(ProceedingJoinPoint thisJoinPoint, MyStructure myStructure) throws Throwable {
writeLock.lock();
log("entering write section", myStructure);
try {
Thread.sleep(1000);
thisJoinPoint.proceed();
} finally {
log("exiting write section", myStructure);
writeLock.unlock();
}
}
@Around("target(myStructure) && execution(void read(int))")
public void ReadPointcutBefore(ProceedingJoinPoint thisJoinPoint, MyStructure myStructure) throws Throwable {
readLock.lock();
log("entering read section", myStructure);
try {
Thread.sleep(1000);
thisJoinPoint.proceed();
} finally {
log("exiting read section", myStructure);
readLock.unlock();
}
}
private static void log(String message, Object targetObject) {
System.out.printf(
"%8d ms | %-25s | %-17s | %s%n",
System.currentTimeMillis() - startTime,
Thread.currentThread(),
targetObject,
message
);
}
}
Sample log output:
4 ms | Thread[Thread-3,5,main] | MyStructure[Two] | entering write section
4 ms | Thread[Thread-6,5,main] | MyStructure[One] | entering read section
4 ms | Thread[Thread-8,5,main] | MyStructure[One] | entering read section
4 ms | Thread[Thread-4,5,main] | MyStructure[One] | entering read section
4 ms | Thread[Thread-10,5,main] | MyStructure[One] | entering read section
1019 ms | Thread[Thread-3,5,main] | MyStructure[Two] | exiting write section
1020 ms | Thread[Thread-8,5,main] | MyStructure[One] | exiting read section
1020 ms | Thread[Thread-4,5,main] | MyStructure[One] | exiting read section
1020 ms | Thread[Thread-11,5,main] | MyStructure[Two] | entering read section
1020 ms | Thread[Thread-5,5,main] | MyStructure[Two] | entering read section
1020 ms | Thread[Thread-6,5,main] | MyStructure[One] | exiting read section
1020 ms | Thread[Thread-10,5,main] | MyStructure[One] | exiting read section
1025 ms | Thread[Thread-2,5,main] | MyStructure[One] | entering write section
2023 ms | Thread[Thread-11,5,main] | MyStructure[Two] | exiting read section
2024 ms | Thread[Thread-5,5,main] | MyStructure[Two] | exiting read section
2025 ms | Thread[Thread-1,5,main] | MyStructure[Two] | entering write section
2026 ms | Thread[Thread-2,5,main] | MyStructure[One] | exiting write section
2026 ms | Thread[Thread-0,5,main] | MyStructure[One] | entering write section
3026 ms | Thread[Thread-1,5,main] | MyStructure[Two] | exiting write section
3026 ms | Thread[Thread-7,5,main] | MyStructure[Two] | entering read section
3026 ms | Thread[Thread-9,5,main] | MyStructure[Two] | entering read section
3028 ms | Thread[Thread-0,5,main] | MyStructure[One] | exiting write section
4028 ms | Thread[Thread-7,5,main] | MyStructure[Two] | exiting read section
4029 ms | Thread[Thread-9,5,main] | MyStructure[Two] | exiting read section
