ReentrantReadWriteLock with AspectJ pointcut for every initialized type of MyStructure
0
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?
java concurrency aop aspectj pointcut
edited Dec 7 '18 at 23:23
halfer
14.4k758109
asked Nov 12 '18 at 22:23
Mixalis NavridisMixalis Navridis
428
add a comment |
0
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?
java concurrency aop aspectj pointcut
edited Dec 7 '18 at 23:23
halfer
14.4k758109
asked Nov 12 '18 at 22:23
Mixalis NavridisMixalis Navridis
428
add a comment |
0
0
0
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?
java concurrency aop aspectj pointcut
edited Dec 7 '18 at 23:23
halfer
14.4k758109
asked Nov 12 '18 at 22:23
Mixalis NavridisMixalis Navridis
428
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?
java concurrency aop aspectj pointcut
java concurrency aop aspectj pointcut
edited Dec 7 '18 at 23:23
halfer
14.4k758109
asked Nov 12 '18 at 22:23
Mixalis NavridisMixalis Navridis
428
edited Dec 7 '18 at 23:23
halfer
14.4k758109
asked Nov 12 '18 at 22:23
Mixalis NavridisMixalis Navridis
428
edited Dec 7 '18 at 23:23
halfer
14.4k758109
edited Dec 7 '18 at 23:23
halfer
14.4k758109
edited Dec 7 '18 at 23:23
halfer
14.4k758109
14.4k758109
asked Nov 12 '18 at 22:23
Mixalis NavridisMixalis Navridis
428
asked Nov 12 '18 at 22:23
Mixalis NavridisMixalis Navridis
428
asked Nov 12 '18 at 22:23
Mixalis NavridisMixalis Navridis
428
428
add a comment |
add a comment |
1 Answer
1
active
oldest
votes
1
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
Writeris already aThreadsubclass, 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
JoinPointpointcutbecause 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
Objectand 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
Readerclass and use it in order to show the difference between reentrant read vs write locks. - I also took care of making
MyStructureinstances 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
MyStructurebefore writing into, I made sure thatMyStructuregets 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
answered Nov 22 '18 at 8:19
kriegaexkriegaex
30.8k363100
Minor, unrelated to aspects comment. The idiom (I think) is to acquire alock, and then enter thetry/finally, nottry, then acquire, thenfinally. The idea being that if an exception (esp.InterruptedException) is thrown while acquiring, you do not want to enter the finally clause and release a lock you never successfully acquired in the first place. Please correct me if I'm mistaken.
– GPI
Nov 22 '18 at 8:47
I know that. The try-finally is for callingproceedas well or whatever else could go wrong and I did not want to nest it. You cannot look at code inside an aspect from a non-aspect perspective only. BTW,proceed()can throwThrowable, but whatever happens, the finally block should execute.
– kriegaex
Nov 22 '18 at 14:10
"whatever happens, the finally block should execute". This is where I disagree. If the lock was successfully acquired then the finally block should execute no matter what. But if the lock was not acquired, then the finally block must not be executed (that would release a lock that was never acquired. If your lock is semaphore based, it would release permits that never meant to exist). By placinglock.lock()inside the try, you open up the possibility of failing to acquire and releasing nonetheless, which will at best fail, and at worse trigger unspecified behavior.
– GPI
Nov 23 '18 at 11:25
Yesterday I looked at too quickly, my bad. I am stupid and ugly and you are right. I updated my code accordingly, moving the lock operations to before the respective 'try' blocks. Thanks for your valuable comments! :-)
– kriegaex
Nov 23 '18 at 11:44
Not an issue. I had a feeling we were not understanding each other. Very nice answer, by the way, +1.
– GPI
Nov 23 '18 at 15:51
add a comment |
Your Answer
StackExchange.ifUsing("editor", function () {
StackExchange.using("externalEditor", function () {
StackExchange.using("snippets", function () {
StackExchange.snippets.init();
});
});
}, "code-snippets");
StackExchange.ready(function() {
var channelOptions = {
tags: "".split(" "),
id: "1"
};
initTagRenderer("".split(" "), "".split(" "), channelOptions);
StackExchange.using("externalEditor", function() {
// Have to fire editor after snippets, if snippets enabled
if (StackExchange.settings.snippets.snippetsEnabled) {
StackExchange.using("snippets", function() {
createEditor();
});
}
else {
createEditor();
}
});
function createEditor() {
StackExchange.prepareEditor({
heartbeatType: 'answer',
autoActivateHeartbeat: false,
convertImagesToLinks: true,
noModals: true,
showLowRepImageUploadWarning: true,
reputationToPostImages: 10,
bindNavPrevention: true,
postfix: "",
imageUploader: {
brandingHtml: "Powered by u003ca class="icon-imgur-white" href="https://imgur.com/"u003eu003c/au003e",
contentPolicyHtml: "User contributions licensed under u003ca href="https://creativecommons.org/licenses/by-sa/3.0/"u003ecc by-sa 3.0 with attribution requiredu003c/au003e u003ca href="https://stackoverflow.com/legal/content-policy"u003e(content policy)u003c/au003e",
allowUrls: true
},
onDemand: true,
discardSelector: ".discard-answer"
,immediatelyShowMarkdownHelp:true
});
}
});
draft saved
draft discarded
Sign up or log in
StackExchange.ready(function () {
StackExchange.helpers.onClickDraftSave('#login-link');
});
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
StackExchange.ready(
function () {
StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fstackoverflow.com%2fquestions%2f53270967%2freentrantreadwritelock-with-aspectj-pointcut-for-every-initialized-type-of-mystr%23new-answer', 'question_page');
}
);
Post as a guest
Required, but never shown
1 Answer
1
active
oldest
votes
1 Answer
1
active
oldest
votes
active
oldest
votes
active
oldest
votes
1
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
Writeris already aThreadsubclass, 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
JoinPointpointcutbecause 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
Objectand 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
Readerclass and use it in order to show the difference between reentrant read vs write locks. - I also took care of making
MyStructureinstances 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
MyStructurebefore writing into, I made sure thatMyStructuregets 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
answered Nov 22 '18 at 8:19
kriegaexkriegaex
30.8k363100
Minor, unrelated to aspects comment. The idiom (I think) is to acquire alock, and then enter thetry/finally, nottry, then acquire, thenfinally. The idea being that if an exception (esp.InterruptedException) is thrown while acquiring, you do not want to enter the finally clause and release a lock you never successfully acquired in the first place. Please correct me if I'm mistaken.
– GPI
Nov 22 '18 at 8:47
I know that. The try-finally is for callingproceedas well or whatever else could go wrong and I did not want to nest it. You cannot look at code inside an aspect from a non-aspect perspective only. BTW,proceed()can throwThrowable, but whatever happens, the finally block should execute.
– kriegaex
Nov 22 '18 at 14:10
"whatever happens, the finally block should execute". This is where I disagree. If the lock was successfully acquired then the finally block should execute no matter what. But if the lock was not acquired, then the finally block must not be executed (that would release a lock that was never acquired. If your lock is semaphore based, it would release permits that never meant to exist). By placinglock.lock()inside the try, you open up the possibility of failing to acquire and releasing nonetheless, which will at best fail, and at worse trigger unspecified behavior.
– GPI
Nov 23 '18 at 11:25
Yesterday I looked at too quickly, my bad. I am stupid and ugly and you are right. I updated my code accordingly, moving the lock operations to before the respective 'try' blocks. Thanks for your valuable comments! :-)
– kriegaex
Nov 23 '18 at 11:44
Not an issue. I had a feeling we were not understanding each other. Very nice answer, by the way, +1.
– GPI
Nov 23 '18 at 15:51
add a comment |
1
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
Writeris already aThreadsubclass, 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
JoinPointpointcutbecause 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
Objectand 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
Readerclass and use it in order to show the difference between reentrant read vs write locks. - I also took care of making
MyStructureinstances 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
MyStructurebefore writing into, I made sure thatMyStructuregets 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
answered Nov 22 '18 at 8:19
kriegaexkriegaex
30.8k363100
Minor, unrelated to aspects comment. The idiom (I think) is to acquire alock, and then enter thetry/finally, nottry, then acquire, thenfinally. The idea being that if an exception (esp.InterruptedException) is thrown while acquiring, you do not want to enter the finally clause and release a lock you never successfully acquired in the first place. Please correct me if I'm mistaken.
– GPI
Nov 22 '18 at 8:47
I know that. The try-finally is for callingproceedas well or whatever else could go wrong and I did not want to nest it. You cannot look at code inside an aspect from a non-aspect perspective only. BTW,proceed()can throwThrowable, but whatever happens, the finally block should execute.
– kriegaex
Nov 22 '18 at 14:10
"whatever happens, the finally block should execute". This is where I disagree. If the lock was successfully acquired then the finally block should execute no matter what. But if the lock was not acquired, then the finally block must not be executed (that would release a lock that was never acquired. If your lock is semaphore based, it would release permits that never meant to exist). By placinglock.lock()inside the try, you open up the possibility of failing to acquire and releasing nonetheless, which will at best fail, and at worse trigger unspecified behavior.
– GPI
Nov 23 '18 at 11:25
Yesterday I looked at too quickly, my bad. I am stupid and ugly and you are right. I updated my code accordingly, moving the lock operations to before the respective 'try' blocks. Thanks for your valuable comments! :-)
– kriegaex
Nov 23 '18 at 11:44
Not an issue. I had a feeling we were not understanding each other. Very nice answer, by the way, +1.
– GPI
Nov 23 '18 at 15:51
add a comment |
1
1
1
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
Writeris already aThreadsubclass, 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
JoinPointpointcutbecause 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
Objectand 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
Readerclass and use it in order to show the difference between reentrant read vs write locks. - I also took care of making
MyStructureinstances 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
MyStructurebefore writing into, I made sure thatMyStructuregets 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
answered Nov 22 '18 at 8:19
kriegaexkriegaex
30.8k363100
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
Writeris already aThreadsubclass, 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
JoinPointpointcutbecause 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
Objectand 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
Readerclass and use it in order to show the difference between reentrant read vs write locks. - I also took care of making
MyStructureinstances 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
MyStructurebefore writing into, I made sure thatMyStructuregets 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
answered Nov 22 '18 at 8:19
kriegaexkriegaex
30.8k363100
edited Nov 23 '18 at 11:42
answered Nov 22 '18 at 8:19
kriegaexkriegaex
30.8k363100
answered Nov 22 '18 at 8:19
kriegaexkriegaex
30.8k363100
answered Nov 22 '18 at 8:19
kriegaexkriegaex
30.8k363100
30.8k363100
Minor, unrelated to aspects comment. The idiom (I think) is to acquire alock, and then enter thetry/finally, nottry, then acquire, thenfinally. The idea being that if an exception (esp.InterruptedException) is thrown while acquiring, you do not want to enter the finally clause and release a lock you never successfully acquired in the first place. Please correct me if I'm mistaken.
– GPI
Nov 22 '18 at 8:47
I know that. The try-finally is for callingproceedas well or whatever else could go wrong and I did not want to nest it. You cannot look at code inside an aspect from a non-aspect perspective only. BTW,proceed()can throwThrowable, but whatever happens, the finally block should execute.
– kriegaex
Nov 22 '18 at 14:10
"whatever happens, the finally block should execute". This is where I disagree. If the lock was successfully acquired then the finally block should execute no matter what. But if the lock was not acquired, then the finally block must not be executed (that would release a lock that was never acquired. If your lock is semaphore based, it would release permits that never meant to exist). By placinglock.lock()inside the try, you open up the possibility of failing to acquire and releasing nonetheless, which will at best fail, and at worse trigger unspecified behavior.
– GPI
Nov 23 '18 at 11:25
Yesterday I looked at too quickly, my bad. I am stupid and ugly and you are right. I updated my code accordingly, moving the lock operations to before the respective 'try' blocks. Thanks for your valuable comments! :-)
– kriegaex
Nov 23 '18 at 11:44
Not an issue. I had a feeling we were not understanding each other. Very nice answer, by the way, +1.
– GPI
Nov 23 '18 at 15:51
add a comment |
Minor, unrelated to aspects comment. The idiom (I think) is to acquire alock, and then enter thetry/finally, nottry, then acquire, thenfinally. The idea being that if an exception (esp.InterruptedException) is thrown while acquiring, you do not want to enter the finally clause and release a lock you never successfully acquired in the first place. Please correct me if I'm mistaken.
– GPI
Nov 22 '18 at 8:47
I know that. The try-finally is for callingproceedas well or whatever else could go wrong and I did not want to nest it. You cannot look at code inside an aspect from a non-aspect perspective only. BTW,proceed()can throwThrowable, but whatever happens, the finally block should execute.
– kriegaex
Nov 22 '18 at 14:10
"whatever happens, the finally block should execute". This is where I disagree. If the lock was successfully acquired then the finally block should execute no matter what. But if the lock was not acquired, then the finally block must not be executed (that would release a lock that was never acquired. If your lock is semaphore based, it would release permits that never meant to exist). By placinglock.lock()inside the try, you open up the possibility of failing to acquire and releasing nonetheless, which will at best fail, and at worse trigger unspecified behavior.
– GPI
Nov 23 '18 at 11:25
Yesterday I looked at too quickly, my bad. I am stupid and ugly and you are right. I updated my code accordingly, moving the lock operations to before the respective 'try' blocks. Thanks for your valuable comments! :-)
– kriegaex
Nov 23 '18 at 11:44
Not an issue. I had a feeling we were not understanding each other. Very nice answer, by the way, +1.
– GPI
Nov 23 '18 at 15:51
Minor, unrelated to aspects comment. The idiom (I think) is to acquire a
lock, and then enter the try/finally, not try, then acquire, then finally. The idea being that if an exception (esp. InterruptedException) is thrown while acquiring, you do not want to enter the finally clause and release a lock you never successfully acquired in the first place. Please correct me if I'm mistaken.– GPI
Nov 22 '18 at 8:47
Minor, unrelated to aspects comment. The idiom (I think) is to acquire a
lock, and then enter the try/finally, not try, then acquire, then finally. The idea being that if an exception (esp. InterruptedException) is thrown while acquiring, you do not want to enter the finally clause and release a lock you never successfully acquired in the first place. Please correct me if I'm mistaken.– GPI
Nov 22 '18 at 8:47
I know that. The try-finally is for calling
proceed as well or whatever else could go wrong and I did not want to nest it. You cannot look at code inside an aspect from a non-aspect perspective only. BTW, proceed() can throw Throwable, but whatever happens, the finally block should execute.– kriegaex
Nov 22 '18 at 14:10
I know that. The try-finally is for calling
proceed as well or whatever else could go wrong and I did not want to nest it. You cannot look at code inside an aspect from a non-aspect perspective only. BTW, proceed() can throw Throwable, but whatever happens, the finally block should execute.– kriegaex
Nov 22 '18 at 14:10
"whatever happens, the finally block should execute". This is where I disagree. If the lock was successfully acquired then the finally block should execute no matter what. But if the lock was not acquired, then the finally block must not be executed (that would release a lock that was never acquired. If your lock is semaphore based, it would release permits that never meant to exist). By placing
lock.lock() inside the try, you open up the possibility of failing to acquire and releasing nonetheless, which will at best fail, and at worse trigger unspecified behavior.– GPI
Nov 23 '18 at 11:25
"whatever happens, the finally block should execute". This is where I disagree. If the lock was successfully acquired then the finally block should execute no matter what. But if the lock was not acquired, then the finally block must not be executed (that would release a lock that was never acquired. If your lock is semaphore based, it would release permits that never meant to exist). By placing
lock.lock() inside the try, you open up the possibility of failing to acquire and releasing nonetheless, which will at best fail, and at worse trigger unspecified behavior.– GPI
Nov 23 '18 at 11:25
Yesterday I looked at too quickly, my bad. I am stupid and ugly and you are right. I updated my code accordingly, moving the lock operations to before the respective 'try' blocks. Thanks for your valuable comments! :-)
– kriegaex
Nov 23 '18 at 11:44
Yesterday I looked at too quickly, my bad. I am stupid and ugly and you are right. I updated my code accordingly, moving the lock operations to before the respective 'try' blocks. Thanks for your valuable comments! :-)
– kriegaex
Nov 23 '18 at 11:44
Not an issue. I had a feeling we were not understanding each other. Very nice answer, by the way, +1.
– GPI
Nov 23 '18 at 15:51
Not an issue. I had a feeling we were not understanding each other. Very nice answer, by the way, +1.
– GPI
Nov 23 '18 at 15:51
add a comment |
draft saved
draft discarded
Thanks for contributing an answer to Stack Overflow!
- Please be sure to answer the question. Provide details and share your research!
But avoid …
- Asking for help, clarification, or responding to other answers.
- Making statements based on opinion; back them up with references or personal experience.
To learn more, see our tips on writing great answers.
draft saved
draft discarded
Sign up or log in
StackExchange.ready(function () {
StackExchange.helpers.onClickDraftSave('#login-link');
});
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
StackExchange.ready(
function () {
StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fstackoverflow.com%2fquestions%2f53270967%2freentrantreadwritelock-with-aspectj-pointcut-for-every-initialized-type-of-mystr%23new-answer', 'question_page');
}
);
Post as a guest
Required, but never shown
Sign up or log in
StackExchange.ready(function () {
StackExchange.helpers.onClickDraftSave('#login-link');
});
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
Sign up or log in
StackExchange.ready(function () {
StackExchange.helpers.onClickDraftSave('#login-link');
});
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
Sign up or log in
StackExchange.ready(function () {
StackExchange.helpers.onClickDraftSave('#login-link');
});
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
