[jboss-cvs] JBossAS SVN: r78603 - projects/aop/trunk/aop/src/main/org/jboss/aop.

[jboss-cvs-commits at lists.jboss.org]

Author: flavia.rainone at jboss.com
Date: 2008-09-16 20:36:25 -0400 (Tue, 16 Sep 2008)
New Revision: 78603

Modified:
   projects/aop/trunk/aop/src/main/org/jboss/aop/AspectManager.java
Log:
[JBAOP-629] Added write lock to the bindingCollection on the addBinding operation.

Modified: projects/aop/trunk/aop/src/main/org/jboss/aop/AspectManager.java
===================================================================
--- projects/aop/trunk/aop/src/main/org/jboss/aop/AspectManager.java	2008-09-16 20:49:16 UTC (rev 78602)
+++ projects/aop/trunk/aop/src/main/org/jboss/aop/AspectManager.java	2008-09-17 00:36:25 UTC (rev 78603)
@@ -1397,27 +1397,43 @@
    {
       Set<Advisor> affectedAdvisors = null;
       AdviceBinding removedBinding = null;
-      synchronized(this)
+      // this locked variable is used in order to avoid breaking the try finally block in two pieces
+      boolean locked = false;
+      try
       {
-         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>();
-         updateAdvisorsForAddedBinding(binding, handledAdvisors);
+         // EXTREMELY IMPORTANT: get this' lock before the bindingCollection's, or
+         // we will end up with a deadlock
+         synchronized(this)
+         {
+            bindingCollection.lockWrite();
+            locked = true;
+            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>();
+            updateAdvisorsForAddedBinding(binding, handledAdvisors);
 
-         if (affectedAdvisors != null && affectedAdvisors.size() > 0)
-         {
-            for (Advisor advisor : affectedAdvisors)
+            if (affectedAdvisors != null && affectedAdvisors.size() > 0)
             {
-               if (isAdvisorRegistered(advisor))
-                  advisor.removeAdviceBinding(removedBinding);
+               for (Advisor advisor : affectedAdvisors)
+               {
+                  if (isAdvisorRegistered(advisor))
+                     advisor.removeAdviceBinding(removedBinding);
+               }
             }
          }
+         this.dynamicStrategy.interceptorChainsUpdated();
       }
-      this.dynamicStrategy.interceptorChainsUpdated();
+      finally
+      {
+         if (locked)
+         {
+            bindingCollection.unlockWrite();
+         }
+      }
    }