[jboss-cvs] JBossAS SVN: r87257 - projects/aop/trunk/aop/src/main/java/org/jboss/aop.
jboss-cvs-commits at lists.jboss.org
jboss-cvs-commits at lists.jboss.org
Tue Apr 14 01:17:18 EDT 2009
Author: flavia.rainone at jboss.com
Date: 2009-04-14 01:17:17 -0400 (Tue, 14 Apr 2009)
New Revision: 87257
Modified:
projects/aop/trunk/aop/src/main/java/org/jboss/aop/AspectManager.java
projects/aop/trunk/aop/src/main/java/org/jboss/aop/Domain.java
projects/aop/trunk/aop/src/main/java/org/jboss/aop/InstanceDomain.java
Log:
[JBAOP-713] The synchronized blocks have been replaced by use of the AOPLock only. The reason for this is that using double synchronization blocks was the same as not getting the benefits of readWrite lock:
synchronized{lock.lockWrite()....}
synchronized{lock.lockRead()....}
is the same as using synchronized{}.
The use of synchronized was responsible for the deadlock scenarios we've been seeing.
Modified: projects/aop/trunk/aop/src/main/java/org/jboss/aop/AspectManager.java
===================================================================
--- projects/aop/trunk/aop/src/main/java/org/jboss/aop/AspectManager.java 2009-04-14 03:06:58 UTC (rev 87256)
+++ projects/aop/trunk/aop/src/main/java/org/jboss/aop/AspectManager.java 2009-04-14 05:17:17 UTC (rev 87257)
@@ -163,6 +163,12 @@
/** ClassExpressions built from ignore. Maintained by top-level AspectManager */
protected ClassExpression[] ignoreExpressions = new ClassExpression[0];
+ /**
+ * This lock synchronizes weaving process with operations that change collections
+ * read during weaving.
+ * For weaving operations, use lock.lockRead(). For operations that change the
+ * collections used during weaving, use lock.lockWrite().
+ */
protected static AOPLock lock = new AOPLock();
protected volatile LinkedHashMap<String, ClassMetaDataBinding> classMetaData = UnmodifiableEmptyCollections.EMPTY_LINKED_HASHMAP;
@@ -726,7 +732,7 @@
return advisor;
}
- public synchronized void initialiseClassAdvisor(Class<?> clazz, ClassAdvisor advisor)
+ public void initialiseClassAdvisor(Class<?> clazz, ClassAdvisor advisor)
{
// avoiding deadlock. Other threads first get the bindignCollection lock
// and then the advisors
@@ -1044,34 +1050,32 @@
{
return null;
}
- synchronized(this){
- lock.lockRead();
- try
+ lock.lockRead();
+ try
+ {
+ if (weavingStrategy == null)
{
- if (weavingStrategy == null)
+ if (TransformerCommon.isCompileTime())
{
- if (TransformerCommon.isCompileTime())
- {
- weavingStrategy = new ClassicWeavingStrategy();
- }
- else if(InstrumentorFactory.getInstrumentor(this,dynamicStrategy.getJoinpointClassifier())
- instanceof GeneratedAdvisorInstrumentor)
- {
- weavingStrategy = new SuperClassesFirstWeavingStrategy();
- }
- else
- {
- weavingStrategy = new ClassicWeavingStrategy();
- }
+ weavingStrategy = new ClassicWeavingStrategy();
}
-
- return weavingStrategy.translate(this, className, loader, classfileBuffer);
+ else if(InstrumentorFactory.getInstrumentor(this,dynamicStrategy.getJoinpointClassifier())
+ instanceof GeneratedAdvisorInstrumentor)
+ {
+ weavingStrategy = new SuperClassesFirstWeavingStrategy();
+ }
+ else
+ {
+ weavingStrategy = new ClassicWeavingStrategy();
+ }
}
- finally
- {
- lock.unlockRead();
- }
+
+ return weavingStrategy.translate(this, className, loader, classfileBuffer);
}
+ finally
+ {
+ lock.unlockRead();
+ }
}
catch (Exception e)
{
@@ -1327,15 +1331,31 @@
*/
public void removePointcut(String name)
{
- bindingCollection.removePointcut(name);
+ lock.lockWrite();
+ try
+ {
+ bindingCollection.removePointcut(name);
+ }
+ finally
+ {
+ lock.unlockWrite();
+ }
}
/**
* Add an interceptor pointcut with a given name
*/
- public synchronized void addPointcut(Pointcut pointcut)
+ public void addPointcut(Pointcut pointcut)
{
- bindingCollection.add(pointcut, this);
+ lock.lockWrite();
+ try
+ {
+ bindingCollection.add(pointcut, this);
+ }
+ finally
+ {
+ lock.unlockWrite();
+ }
}
public boolean isExecution()
@@ -1376,7 +1396,7 @@
/**
* Remove an interceptor pointcut with a given name
*/
- public synchronized void removeBinding(String name)
+ public void removeBinding(String name)
{
lock.lockWrite();
try
@@ -1394,7 +1414,7 @@
}
}
- public synchronized void removeBindings(ArrayList<String> binds)
+ public void removeBindings(ArrayList<String> binds)
{
clearUnregisteredClassLoaders();
@@ -1453,20 +1473,12 @@
{
Set<Advisor> affectedAdvisors = null;
AdviceBinding removedBinding = null;
- // this locked variable is used in order to avoid breaking the try finally block in two pieces
- boolean locked = false;
+ lock.lockWrite();
try
{
- // EXTREMELY IMPORTANT: get this' lock before the bindingCollection's, or
- // we will end up with a deadlock
- synchronized(this)
- {
- lock.lockWrite();
- locked = true;
- removedBinding = internalRemoveBinding(binding.getName());
- affectedAdvisors = removedBinding == null ? null : new HashSet<Advisor>(removedBinding.getAdvisors());
- bindingCollection.add(binding, this);
- }
+ removedBinding = internalRemoveBinding(binding.getName());
+ affectedAdvisors = removedBinding == null ? null : new HashSet<Advisor>(removedBinding.getAdvisors());
+ bindingCollection.add(binding, this);
synchronized (advisors)
{
Set<Advisor> handledAdvisors = new HashSet<Advisor>();
@@ -1485,10 +1497,7 @@
}
finally
{
- if (locked)
- {
- lock.unlockWrite();
- }
+ lock.unlockWrite();
}
}
@@ -1640,14 +1649,22 @@
/**
* Register an introduction pointcut
*/
- public synchronized void addInterfaceIntroduction(InterfaceIntroduction pointcut)
+ public void addInterfaceIntroduction(InterfaceIntroduction pointcut)
{
- removeInterfaceIntroduction(pointcut.getName());
- initInterfaceIntroductionsMap();
- synchronized (interfaceIntroductions)
+ lock.lockWrite();
+ try
{
- interfaceIntroductions.put(pointcut.getName(), pointcut);
+ removeInterfaceIntroduction(pointcut.getName());
+ initInterfaceIntroductionsMap();
+ synchronized (interfaceIntroductions)
+ {
+ interfaceIntroductions.put(pointcut.getName(), pointcut);
+ }
}
+ finally
+ {
+ lock.unlockWrite();
+ }
}
/**
@@ -1655,12 +1672,20 @@
*/
public void removeInterfaceIntroduction(String name)
{
- synchronized (interfaceIntroductions)
+ lock.lockWrite();
+ try
{
- InterfaceIntroduction pointcut = interfaceIntroductions.remove(name);
- if (pointcut == null) return;
- pointcut.clearAdvisors();
+ synchronized (interfaceIntroductions)
+ {
+ InterfaceIntroduction pointcut = interfaceIntroductions.remove(name);
+ if (pointcut == null) return;
+ pointcut.clearAdvisors();
+ }
}
+ finally
+ {
+ lock.unlockWrite();
+ }
}
/**
@@ -1677,14 +1702,22 @@
/**
* Register an introduction pointcut
*/
- public synchronized void addArrayReplacement(ArrayReplacement pointcut)
+ public void addArrayReplacement(ArrayReplacement pointcut)
{
- removeArrayReplacement(pointcut.getName());
- initArrayReplacementMap();
- synchronized (arrayReplacements)
+ lock.lockWrite();
+ try
{
- arrayReplacements.put(pointcut.getName(), pointcut);
+ removeArrayReplacement(pointcut.getName());
+ initArrayReplacementMap();
+ synchronized (arrayReplacements)
+ {
+ arrayReplacements.put(pointcut.getName(), pointcut);
+ }
}
+ finally
+ {
+ lock.unlockWrite();
+ }
}
/**
@@ -1692,10 +1725,18 @@
*/
public void removeArrayReplacement(String name)
{
- synchronized (arrayReplacements)
+ lock.lockWrite();
+ try
{
- arrayReplacements.remove(name);
+ synchronized (arrayReplacements)
+ {
+ arrayReplacements.remove(name);
+ }
}
+ finally
+ {
+ lock.unlockWrite();
+ }
}
/**
@@ -1712,15 +1753,23 @@
/**
* Register an introduction pointcut
*/
- public synchronized void addArrayBinding(ArrayBinding binding)
+ public void addArrayBinding(ArrayBinding binding)
{
- removeArrayBinding(binding.getName());
- initArrayBindingMap();
- synchronized (arrayBindings)
+ lock.lockWrite();
+ try
{
- arrayBindings.put(binding.getName(), binding);
- ArrayAdvisor.addBinding(binding);
+ removeArrayBinding(binding.getName());
+ initArrayBindingMap();
+ synchronized (arrayBindings)
+ {
+ arrayBindings.put(binding.getName(), binding);
+ ArrayAdvisor.addBinding(binding);
+ }
}
+ finally
+ {
+ lock.unlockWrite();
+ }
}
/**
@@ -1728,27 +1777,43 @@
*/
public void removeArrayBinding(String name)
{
- synchronized (arrayBindings)
+ lock.lockWrite();
+ try
{
- ArrayBinding pointcut = arrayBindings.remove(name);
- if (pointcut == null) return;
- ArrayAdvisor.removeBinding(pointcut);
+ synchronized (arrayBindings)
+ {
+ ArrayBinding pointcut = arrayBindings.remove(name);
+ if (pointcut == null) return;
+ ArrayAdvisor.removeBinding(pointcut);
+ }
}
+ finally
+ {
+ lock.unlockWrite();
+ }
}
/**
* Register an annotation introduction
*/
- public synchronized void addAnnotationIntroduction(AnnotationIntroduction pointcut)
+ public void addAnnotationIntroduction(AnnotationIntroduction pointcut)
{
- String name = pointcut.getOriginalAnnotationExpr() + pointcut.getOriginalExpression();
- removeAnnotationIntroduction(pointcut);
- initAnnotationIntroductionsMap();
- synchronized (annotationIntroductions)
+ lock.lockWrite();
+ try
{
- annotationIntroductions.put(name, pointcut);
+ String name = pointcut.getOriginalAnnotationExpr() + pointcut.getOriginalExpression();
+ removeAnnotationIntroduction(pointcut);
+ initAnnotationIntroductionsMap();
+ synchronized (annotationIntroductions)
+ {
+ annotationIntroductions.put(name, pointcut);
+ }
}
+ finally
+ {
+ lock.unlockWrite();
+ }
}
/**
@@ -1756,11 +1821,19 @@
*/
public void removeAnnotationIntroduction(AnnotationIntroduction pointcut)
{
- String name = pointcut.getOriginalAnnotationExpr() + pointcut.getOriginalExpression();
- synchronized (annotationIntroductions)
+ lock.lockWrite();
+ try
{
- annotationIntroductions.remove(name);
+ String name = pointcut.getOriginalAnnotationExpr() + pointcut.getOriginalExpression();
+ synchronized (annotationIntroductions)
+ {
+ annotationIntroductions.remove(name);
+ }
}
+ finally
+ {
+ lock.unlockWrite();
+ }
}
public List<AnnotationIntroduction> getAnnotationIntroductions()
@@ -1771,29 +1844,45 @@
}
}
- public synchronized void addDeclare(DeclareDef declare)
+ public void addDeclare(DeclareDef declare)
{
- removeDeclare(declare.getName());
- initDeclaresMap();
- synchronized (declares)
+ lock.lockWrite();
+ try
{
- declares.put(declare.getName(), declare);
+ removeDeclare(declare.getName());
+ initDeclaresMap();
+ synchronized (declares)
+ {
+ declares.put(declare.getName(), declare);
+ }
+ if (declare.isPointcut())
+ {
+ PointcutStats stats;
+ stats = new PointcutStats(declare.getAst(), manager);
+ stats.matches();
+ bindingCollection.updateStats(stats);
+ }
}
- if (declare.isPointcut())
+ finally
{
- PointcutStats stats;
- stats = new PointcutStats(declare.getAst(), manager);
- stats.matches();
- bindingCollection.updateStats(stats);
+ lock.unlockWrite();
}
}
public void removeDeclare(String name)
{
- synchronized (declares)
+ lock.lockWrite();
+ try
{
- declares.remove(name);
+ synchronized (declares)
+ {
+ declares.remove(name);
+ }
}
+ finally
+ {
+ lock.unlockWrite();
+ }
}
public Iterator<DeclareDef> getDeclares()
@@ -1835,15 +1924,23 @@
/**
* Register an annotation introduction
*/
- public synchronized void addAnnotationOverride(AnnotationIntroduction pointcut)
+ public void addAnnotationOverride(AnnotationIntroduction pointcut)
{
- String name = pointcut.getOriginalAnnotationExpr() + pointcut.getOriginalExpression();
- initAnnotationOverridesMap();
- synchronized (annotationOverrides)
+ lock.lockWrite();
+ try
{
- annotationOverrides.put(name, pointcut);
+ String name = pointcut.getOriginalAnnotationExpr() + pointcut.getOriginalExpression();
+ initAnnotationOverridesMap();
+ synchronized (annotationOverrides)
+ {
+ annotationOverrides.put(name, pointcut);
+ }
+ updateAdvisorsForAddedAnnotationOverride(pointcut);
}
- updateAdvisorsForAddedAnnotationOverride(pointcut);
+ finally
+ {
+ lock.unlockWrite();
+ }
}
public void updateAdvisorsForAddedAnnotationOverride(AnnotationIntroduction introduction)
@@ -1880,11 +1977,19 @@
*/
public void removeAnnotationOverride(AnnotationIntroduction pointcut)
{
- String name = pointcut.getOriginalAnnotationExpr() + pointcut.getOriginalExpression();
- synchronized (annotationOverrides)
+ lock.lockWrite();
+ try
{
- annotationOverrides.remove(name);
+ String name = pointcut.getOriginalAnnotationExpr() + pointcut.getOriginalExpression();
+ synchronized (annotationOverrides)
+ {
+ annotationOverrides.remove(name);
+ }
}
+ finally
+ {
+ lock.unlockWrite();
+ }
}
public List<AnnotationIntroduction> getAnnotationOverrides()
@@ -1994,23 +2099,39 @@
{
return aspectDefinitions.get(name);
}
-
- public synchronized void addTypedef(Typedef def) throws Exception
+
+ public void addTypedef(Typedef def) throws Exception
{
- removeTypedef(def.getName());
- initTypedefsMap();
- synchronized (typedefs)
+ lock.lockWrite();
+ try
{
- typedefs.put(def.getName(), def);
+ removeTypedef(def.getName());
+ initTypedefsMap();
+ synchronized (typedefs)
+ {
+ typedefs.put(def.getName(), def);
+ }
}
+ finally
+ {
+ lock.unlockWrite();
+ }
}
public void removeTypedef(String name)
{
- synchronized (typedefs)
+ lock.lockWrite();
+ try
{
- typedefs.remove(name);
+ synchronized (typedefs)
+ {
+ typedefs.remove(name);
+ }
}
+ finally
+ {
+ lock.unlockWrite();
+ }
}
public Typedef getTypedef(String name)
@@ -2091,9 +2212,12 @@
/**
* Removes an AdviceBinding without notifying dynamic aop strategy.
+ *
+ * Needs to be called from inside a synchronized lock with the AspectManager.lock.
+ *
* @param name the binding to be removed.
*/
- private synchronized AdviceBinding internalRemoveBinding(String name)
+ private AdviceBinding internalRemoveBinding(String name)
{
AdviceBinding binding = bindingCollection.removeBinding(name);
if (binding == null)
Modified: projects/aop/trunk/aop/src/main/java/org/jboss/aop/Domain.java
===================================================================
--- projects/aop/trunk/aop/src/main/java/org/jboss/aop/Domain.java 2009-04-14 03:06:58 UTC (rev 87256)
+++ projects/aop/trunk/aop/src/main/java/org/jboss/aop/Domain.java 2009-04-14 05:17:17 UTC (rev 87257)
@@ -191,7 +191,7 @@
}
@Override
- public synchronized void addBinding(AdviceBinding binding)
+ public void addBinding(AdviceBinding binding)
{
lock.lockWrite();
try
@@ -207,7 +207,7 @@
}
@Override
- public synchronized void removeBinding(String name)
+ public void removeBinding(String name)
{
lock.lockWrite();
try
@@ -223,7 +223,7 @@
}
@Override
- public synchronized void removeBindings(ArrayList<String> binds)
+ public void removeBindings(ArrayList<String> binds)
{
lock.lockWrite();
try
@@ -250,17 +250,33 @@
}
@Override
- public synchronized void addPointcut(Pointcut pointcut)
+ public void addPointcut(Pointcut pointcut)
{
- hasOwnPointcuts = true;
- super.addPointcut(pointcut);
+ lock.lockWrite();
+ try
+ {
+ hasOwnPointcuts = true;
+ super.addPointcut(pointcut);
+ }
+ finally
+ {
+ lock.unlockWrite();
+ }
}
@Override
public void removePointcut(String name)
{
- super.removePointcut(name);
- hasOwnPointcuts = bindingCollection.hasPointcuts();
+ lock.lockWrite();
+ try
+ {
+ super.removePointcut(name);
+ hasOwnPointcuts = bindingCollection.hasPointcuts();
+ }
+ finally
+ {
+ lock.unlockWrite();
+ }
}
@Override
@@ -336,17 +352,33 @@
}
@Override
- public synchronized void addAnnotationIntroduction(AnnotationIntroduction pointcut)
+ public void addAnnotationIntroduction(AnnotationIntroduction pointcut)
{
- hasOwnAnnotationIntroductions = true;
- super.addAnnotationIntroduction(pointcut);
+ lock.lockWrite();
+ try
+ {
+ hasOwnAnnotationIntroductions = true;
+ super.addAnnotationIntroduction(pointcut);
+ }
+ finally
+ {
+ lock.unlockWrite();
+ }
}
@Override
public void removeAnnotationIntroduction(AnnotationIntroduction pointcut)
{
- super.removeAnnotationIntroduction(pointcut);
- hasOwnAnnotationIntroductions = annotationIntroductions.size() > 0;
+ lock.lockWrite();
+ try
+ {
+ super.removeAnnotationIntroduction(pointcut);
+ hasOwnAnnotationIntroductions = annotationIntroductions.size() > 0;
+ }
+ finally
+ {
+ lock.unlockWrite();
+ }
}
@Override
@@ -384,17 +416,33 @@
}
@Override
- public synchronized void addAnnotationOverride(AnnotationIntroduction pointcut)
+ public void addAnnotationOverride(AnnotationIntroduction pointcut)
{
- hasOwnAnnotationOverrides = true;
- super.addAnnotationOverride(pointcut);
+ lock.lockWrite();
+ try
+ {
+ hasOwnAnnotationOverrides = true;
+ super.addAnnotationOverride(pointcut);
+ }
+ finally
+ {
+ lock.unlockWrite();
+ }
}
@Override
public void removeAnnotationOverride(AnnotationIntroduction pointcut)
{
- super.removeAnnotationOverride(pointcut);
- hasOwnAnnotationOverrides = annotationOverrides.size() > 0;
+ lock.lockWrite();
+ try
+ {
+ super.removeAnnotationOverride(pointcut);
+ hasOwnAnnotationOverrides = annotationOverrides.size() > 0;
+ }
+ finally
+ {
+ lock.unlockWrite();
+ }
}
@Override
@@ -432,17 +480,33 @@
}
@Override
- public synchronized void addInterfaceIntroduction(InterfaceIntroduction pointcut)
+ public void addInterfaceIntroduction(InterfaceIntroduction pointcut)
{
- hasOwnInterfaceIntroductions = true;
- super.addInterfaceIntroduction(pointcut);
+ lock.lockWrite();
+ try
+ {
+ hasOwnInterfaceIntroductions = true;
+ super.addInterfaceIntroduction(pointcut);
+ }
+ finally
+ {
+ lock.unlockWrite();
+ }
}
@Override
public void removeInterfaceIntroduction(String name)
{
- super.removeInterfaceIntroduction(name);
- hasOwnInterfaceIntroductions = interfaceIntroductions.size() > 0;
+ lock.lockWrite();
+ try
+ {
+ super.removeInterfaceIntroduction(name);
+ hasOwnInterfaceIntroductions = interfaceIntroductions.size() > 0;
+ }
+ finally
+ {
+ lock.unlockWrite();
+ }
}
@Override
@@ -481,17 +545,33 @@
}
@Override
- public synchronized void addTypedef(Typedef def) throws Exception
+ public void addTypedef(Typedef def) throws Exception
{
- hasOwnTypedefs = true;
- super.addTypedef(def);
+ lock.lockWrite();
+ try
+ {
+ hasOwnTypedefs = true;
+ super.addTypedef(def);
+ }
+ finally
+ {
+ lock.unlockWrite();
+ }
}
@Override
public void removeTypedef(String name)
{
- super.removeTypedef(name);
- hasOwnTypedefs = typedefs.size() > 0;
+ lock.lockWrite();
+ try
+ {
+ super.removeTypedef(name);
+ hasOwnTypedefs = typedefs.size() > 0;
+ }
+ finally
+ {
+ lock.unlockWrite();
+ }
}
public Map<String, AdviceStack> getInterceptorStacks()
Modified: projects/aop/trunk/aop/src/main/java/org/jboss/aop/InstanceDomain.java
===================================================================
--- projects/aop/trunk/aop/src/main/java/org/jboss/aop/InstanceDomain.java 2009-04-14 03:06:58 UTC (rev 87256)
+++ projects/aop/trunk/aop/src/main/java/org/jboss/aop/InstanceDomain.java 2009-04-14 05:17:17 UTC (rev 87257)
@@ -49,12 +49,20 @@
this.advisor = advisor;
}
- public synchronized void addBinding(AdviceBinding binding)
+ public void addBinding(AdviceBinding binding)
{
- removeBinding(binding.getName());
- bindingCollection.add(binding, this);
- super.addPointcut(binding.getPointcut());
- if (advisor != null) advisor.newBindingAdded();
+ lock.lockWrite();
+ try
+ {
+ removeBinding(binding.getName());
+ bindingCollection.add(binding, this);
+ super.addPointcut(binding.getPointcut());
+ if (advisor != null) advisor.newBindingAdded();
+ }
+ finally
+ {
+ lock.unlockWrite();
+ }
}
public void addClassMetaData(ClassMetaDataBinding meta)
More information about the jboss-cvs-commits
mailing list