Howard,
I think I know what is going on here...
In you following advice:
before() : writeMethods() && cflow(readMethods())
{
System.out.println(thisJoinPoint + " " + _lock + ": releasing
read lock (for upgrade)");
_lock.readLock().unlock();
}
The aspect instance used in the advice corresponds to the 'this' object matching the writeMethod() pointcut, instead of readMethods() as you seem to need.
Here is a possible solution:
pointcut readMethod(Object monitored) : <your regular definition> && this(monitored);
Then you can use the wormhole design pattern to access the right object:
before(Object monitored) : writeMethods() && cflow(readMethods(monitored)) {
Synchronized.aspectOf(monitored)._lock.readLock().unlock();
}
I think this code can be improved upon, perhaps using the worker object creation pattern.
-Ramnivas
Howard Lewis Ship wrote:
I'm seeing something similar. When a method of object A with a read
lock invokes a method on object B with a write lock, I get a failure,
since the advice incorrectly tries to upgrade/downgrade the read lock.
** Start synchronization
execution(void org.apache.tapestry.internal.aspects.SynchronizationTarget.setCounter(int))
java.util.concurrent.locks.ReentrantReadWriteLock@1acd47[Write locks =
0, Read locks = 0]: acquiring write lock
execution(void org.apache.tapestry.internal.aspects.SynchronizationTarget.setCounter(int))
java.util.concurrent.locks.ReentrantReadWriteLock@1acd47[Write locks =
1, Read locks = 0]: releasing write lock
execution(void org.apache.tapestry.internal.aspects.SynchTargetWrapper.run())
java.util.concurrent.locks.ReentrantReadWriteLock@30d82d[Write locks =
0, Read locks = 0]: acquiring read lock
execution(void org.apache.tapestry.internal.aspects.SynchronizationTarget.incrementCounter())
java.util.concurrent.locks.ReentrantReadWriteLock@1acd47[Write locks =
0, Read locks = 0]: releasing read lock (for upgrade)
execution(void org.apache.tapestry.internal.aspects.SynchTargetWrapper.run())
java.util.concurrent.locks.ReentrantReadWriteLock@30d82d[Write locks =
0, Read locks = 1]: acquiring read lock
execution(void org.apache.tapestry.internal.aspects.SynchronizationTarget.incrementCounter())
java.util.concurrent.locks.ReentrantReadWriteLock@1acd47[Write locks =
0, Read locks = 0]: releasing write lock
execution(void org.apache.tapestry.internal.aspects.SynchronizationTarget.incrementCounter())
java.util.concurrent.locks.ReentrantReadWriteLock@1acd47[Write locks =
0, Read locks = 0]: releasing read lock (for upgrade)
execution(void org.apache.tapestry.internal.aspects.SynchTargetWrapper.run())
java.util.concurrent.locks.ReentrantReadWriteLock@30d82d[Write locks =
0, Read locks = 2]: acquiring read lock
execution(void org.apache.tapestry.internal.aspects.SynchTargetWrapper.run())
java.util.concurrent.locks.ReentrantReadWriteLock@30d82d[Write locks =
0, Read locks = 2]: acquiring read lock
execution(void org.apache.tapestry.internal.aspects.SynchTargetWrapper.run())
java.util.concurrent.locks.ReentrantReadWriteLock@30d82d[Write locks =
0, Read locks = 2]: acquiring read lock
execution(void org.apache.tapestry.internal.aspects.SynchronizationTarget.incrementCounter())
java.util.concurrent.locks.ReentrantReadWriteLock@1acd47[Write locks =
0, Read locks = 0]: reacquiring read lock (for downgrade)
execution(void org.apache.tapestry.internal.aspects.SynchronizationTarget.incrementCounter())
java.util.concurrent.locks.ReentrantReadWriteLock@1acd47[Write locks =
0, Read locks = 0]: releasing write lock
execution(void org.apache.tapestry.internal.aspects.SynchronizationTarget.incrementCounter())
java.util.concurrent.locks.ReentrantReadWriteLock@1acd47[Write locks =
0, Read locks = 0]: releasing read lock (for upgrade)
execution(void org.apache.tapestry.internal.aspects.SynchronizationTarget.incrementCounter())
java.util.concurrent.locks.ReentrantReadWriteLock@1acd47[Write locks =
0, Read locks = 0]: releasing read lock (for upgrade)
execution(void org.apache.tapestry.internal.aspects.SynchronizationTarget.incrementCounter())
java.util.concurrent.locks.ReentrantReadWriteLock@1acd47[Write locks =
0, Read locks = 0]: releasing read lock (for upgrade)
execution(void org.apache.tapestry.internal.aspects.SynchTargetWrapper.run())
java.util.concurrent.locks.ReentrantReadWriteLock@30d82d[Write locks =
0, Read locks = 5]: releasing read lock
execution(void org.apache.tapestry.internal.aspects.SynchTargetWrapper.run())
java.util.concurrent.locks.ReentrantReadWriteLock@30d82d[Write locks =
0, Read locks = 5]: acquiring read lock
Exception in thread "Thread-40" java.lang.IllegalMonitorStateException
at java.util.concurrent.locks.ReentrantReadWriteLock$Sync.tryRelease(ReentrantReadWriteLock.java:259)
at java.util.concurrent.locks.AbstractQueuedSynchronizer.release(AbstractQueuedSynchronizer.java:1102)
at java.util.concurrent.locks.ReentrantReadWriteLock$WriteLock.unlock(ReentrantReadWriteLock.java:821)
at org.apache.tapestry.internal.aspects.Synchronization.ajc$after$org_apache_tapestry_internal_aspects_Synchronization$5$796b5f14(Synchronization.aj:97)
at org.apache.tapestry.internal.aspects.SynchronizationTarget.incrementCounter(SynchronizationTarget.java:25)
at org.apache.tapestry.internal.aspects.SynchTargetWrapper.run(SynchTargetWrapper.java:23)
at java.lang.Thread.run(Thread.java:595)
Method SynchTargetWrapper.run() invokes method
SynchronizationTarget.incrementCounter().
Here's the relevant portions of my aspect:
/** Before read lock methods, acquire the read lock. */
before() : readMethods()
{
System.out.println(thisJoinPoint + " " + _lock + ": acquiring
read lock");
_lock.readLock().lock();
}
/** After read lock methods (including thrown exceptions), release
the read lock. */
after() : readMethods()
{
System.out.println(thisJoinPoint + " " + _lock + ": releasing
read lock");
_lock.readLock().unlock();
}
/** Order counts. This advice must precede the standard
writeMethods advice below. */
before() : writeMethods() && cflow(readMethods())
{
System.out.println(thisJoinPoint + " " + _lock + ": releasing
read lock (for upgrade)");
_lock.readLock().unlock();
}
/**
* Before write lock methods, acquire the write lock. Note that
obtaining the write lock will
* block indefinately if the current thread has a read lock, but
we handle that as a special
* case.
*/
before(): writeMethods()
{
System.out.println(thisJoinPoint + " " + _lock + ": acquiring
write lock");
_lock.writeLock().lock();
}
/** And release the write lock after the method completes
(successfully, or with an exception). */
after() : writeMethods()
{
System.out.println(thisJoinPoint + " " + _lock + ": releasing
write lock");
_lock.writeLock().unlock();
}
/**
* After releasing the write lock we may re-acquire the read lock
(if the write method was
* invoked from a read method).
*/
after() : writeMethods() && cflow(readMethods())
{
System.out.println(thisJoinPoint + " " + _lock + ":
reacquiring read lock (for downgrade)");
_lock.readLock().lock();
}
... so, it looks to me like the cflow() is matching methods on
different classes and/or instances despite what Adrian said earlier.
BTW ... is it safe to assume that aspects will be applied in the order
they appear in the source?
On 4/25/06, iyad issa <iyadissa@xxxxxxxxx> wrote:
this was my understanding but when i test this i found there is a cross over
i.e
For
A {
@WriteLock void m1(){}
}
B {
A a = new A();
@ReadLock void m2(){
a.m1();
}
}
now call to a.m1() has been captured in the cflow of m2()
I used ajdt version 1.4.0.20060306054947
I will test this with the latest ajdt and report here the findings
iyad
On 25/04/06, Adrian Colyer <adrian.colyer@xxxxxxxxx> wrote:
This works due to the "implicit scoping" associated with aspect
instantiation models. When you have a perthis(somepc()) aspect, then an
aspect instance is created for each unique object bound to "this" at join
points matched by somepc(). Once the aspect instance has been created, every
advice declaration has an implicit condition 'and-ed' to its pointcut
clause. The implicit condition for the different instantiation models is as
follows:
* perthis -> '&& this(the-this-object)'
* pertarget -> '&& target(the-target-object)'
* percflow -> '&& cflow(the-cflow-initiating-join-point)'
* percflowbelow -> '&&
cflowbelow(the-cflowbelow-initiating-join-point)'
* pertypewithin -> '&& within(the-type)
So for any given aspect instance, readLockMethods() only matches read lock
methods on the advised instance, and writeLockMethods only matches
writeLockMethods on the advised instance too. Therefore you won't get
'crossover' between advised instances whereby calls from one object to
another cause inappropriate lock release.
This is documented in the programming guide here:
http://www.eclipse.org/aspectj/doc/released/progguide/semantics-aspects.html#aspect-instantiation
, and also in the Eclipse AspectJ book (see eg. table 9.1 in chapter 9).
Regards, Adrian.
On 25/04/06, Howard Lewis Ship < hlship@xxxxxxxxx > wrote:
What about the case where you call from a read method in instance A to
a write method in instance B. That looks like it will release the read
lock on A, even though it shouldn't.
On 4/25/06, iyad issa <iyadissa@xxxxxxxxx> wrote:
how about change the before and the after advice for writeLock to the
follwoing
before(): writeLockMethods() && !cflow(readLockMethods()){
_lock.writeLock().lock();
}
before(): writeLockMethods() && cflow(readLockMethods()){
_lock.readLock().unlock();
_lock.writeLock().lock();
}
/** And release the write lock after the method completes
(successfully,
or with an exception). */
after() : writeLockMethods() && !cflow(readLockMethods()){
_lock.writeLock().unlock();
}
after() : writeLockMethods() && cflow(readLockMethods()){
_lock.writeLock().unlock();
_lock.readLock().lock();
}
this should work
iyad
On 24/04/06, Howard Lewis Ship < hlship@xxxxxxxxx> wrote:
I'm writing some synchronization aspects now.
I want to annotation methods with @ReadLock or @WriteLock and have the
annotation manage the lock.
So far, so good:
package org.apache.tapestry.internal.aspects ;
import java.util.concurrent.locks.ReadWriteLock;
import
java.util.concurrent.locks.ReentrantReadWriteLock
;
import
org.apache.tapestry.internal.annotations.ReadLock ;
import
org.apache.tapestry.internal.annotations.WriteLock;
/**
* Associates a {@link
java.util.concurrent.locks.ReadWriteLock} with
an object instance; the
* {@link ReadLock} and {@link WriteLock} annotations drive this.
Methods that have the ReadLock
* annotation witll be advised to obtain and release the read lock
around their execution. Methods
* with the WriteLock annotation will obtain and release the write
lock around their execution.
* Methods with ReadLock that call a WriteLock-ed method (within the
same instance) will release the
* read lock before invoking the WriteLock-ed method.
* <p>
* This aspect also enforces that the annotations are only applied to
instance (not static) methods.
*
* @author Howard M. Lewis Ship
*/
public abstract aspect Synchronization extends AbstractClassTargetting
perthis(readLockMethods() || writeLockMethods())
{
private final ReadWriteLock _lock = new ReentrantReadWriteLock();
declare error :
targetClasses() &&
execution(@(ReadLock || WriteLock) static * *(..)) :
"ReadLock and WriteLock annotations may only be applied to
instance methods.";
declare error :
targetClasses() &&
execution(@ReadLock @WriteLock * *(..)) :
"A method may be annotated with ReadLock or with WriteLock
but not both.";
pointcut readLockMethods() :
targetClasses() &&
execution(@ReadLock * *(..));
pointcut writeLockMethods() :
targetClasses() &&
execution(@WriteLock * *(..));
/** Before read lock methods, acquire the read lock. */
before() : readLockMethods()
{
_lock.readLock().lock();
}
/** After read lock methods (including thrown exceptions), release
the read lock. */
after() : readLockMethods()
{
_lock.readLock().unlock();
}
/**
* Before write lock methods, acquire the write lock. Note that
obtaining the write lock will
* block indefinately if the current thread has a read lock, but
we handle that as a special
* case.
*/
before(): writeLockMethods()
{
_lock.writeLock().lock();
}
/** And release the write lock after the method completes
(successfully, or with an exception). */
after() : writeLockMethods()
{
_lock.writeLock().unlock();
}
}
Here's my new issues:
1. Using perthis() creates the aspect instance dynamically, but I'm
worried that it uses a synchronized block that will serialize my
methods after all the effort I've put in to make them highly parallel
(using the readwrite lock).
2. One very important case is not coverred:
If a method with @ReadLock invokes a method OF THE SAME INSTANCE with
@WriteLock, then we need to release the read lock before we invoke the
write lock method, then re-obtain the read lock afterwards. This
feels like something you would do with cflow(), but I can't figure out
how to bind to an instance, rather than a type.
BTW, I'm finding the combintation of Aspects and Annotations to be
very powerful.
Using annotations to mark types or methods works well in combintaion
with a base aspect that defines an abstract targetClasses() pointcut.
Concrete aspects provide a specific set of classes for targetClasses()
and the annotation matching does the rest.
Thanks in advance!
--
Howard M. Lewis Ship
Independent J2EE / Open-Source Java Consultant
Creator, Jakarta Tapestry
Creator, Jakarta HiveMind
Professional Tapestry training, mentoring, support
and project work. http://howardlewisship.com
_______________________________________________
aspectj-users mailing list
aspectj-users@xxxxxxxxxxx
https://dev.eclipse.org/mailman/listinfo/aspectj-users
_______________________________________________
aspectj-users mailing list
aspectj-users@xxxxxxxxxxx
https://dev.eclipse.org/mailman/listinfo/aspectj-users
--
Howard M. Lewis Ship
Independent J2EE / Open-Source Java Consultant
Creator, Jakarta Tapestry
Creator, Jakarta HiveMind
Professional Tapestry training, mentoring, support
and project work. http://howardlewisship.com
_______________________________________________
aspectj-users mailing list
aspectj-users@xxxxxxxxxxx
https://dev.eclipse.org/mailman/listinfo/aspectj-users
--
-- Adrian
adrian.colyer@xxxxxxxxx
_______________________________________________
aspectj-users mailing list
aspectj-users@xxxxxxxxxxx
https://dev.eclipse.org/mailman/listinfo/aspectj-users
_______________________________________________
aspectj-users mailing list
aspectj-users@xxxxxxxxxxx
https://dev.eclipse.org/mailman/listinfo/aspectj-users
--
Howard M. Lewis Ship
Independent J2EE / Open-Source Java Consultant
Creator, Jakarta Tapestry
Creator, Jakarta HiveMind
Professional Tapestry training, mentoring, support
and project work. http://howardlewisship.com
_______________________________________________
aspectj-users mailing list
aspectj-users@xxxxxxxxxxx
https://dev.eclipse.org/mailman/listinfo/aspectj-users
|
