Fixed a race condition. The dead lock was between the transientActions object vs MavenModuleSet.

 
Found one Java-level deadlock:
=============================
"Handling GET /hudson/job/special_Build/api/json : http-8080-4":
  waiting to lock monitor 0x00000000460dfa98 (object 0x00002aab02808368, a java.util.Vector),
  which is held by "pool-6-thread-250"

"pool-6-thread-250":
  waiting to lock monitor 0x0000000046aca2b0 (object 0x00002aaab7099118, a hudson.maven.MavenModuleSet),
  which is held by "Handling GET /hudson/job/special_Build/api/json : http-8080-4"






"Handling GET /hudson/job/special_Build/api/json : http-8080-4":
        at java.util.Vector.toArray(Vector.java:643)
->      - waiting to lock <0x00002aab02808368> (a java.util.Vector)
        at java.util.Vector.addAll(Vector.java:830)
        - locked <0x00002aab02812710> (a java.util.Vector)
        at hudson.model.AbstractProject.getActions(AbstractProject.java:899)
->      - locked <0x00002aaab7099118> (a hudson.maven.MavenModuleSet)
        at sun.reflect.GeneratedMethodAccessor149.invoke(Unknown Source)
        at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25)
        at java.lang.reflect.Method.invoke(Method.java:597)
        at org.kohsuke.stapler.export.MethodProperty.getValue(MethodProperty.java:43)
        at org.kohsuke.stapler.export.Property.writeTo(Property.java:83)
        at org.kohsuke.stapler.export.Model.writeNestedObjectTo(Model.java:156)
        at org.kohsuke.stapler.export.Model.writeNestedObjectTo(Model.java:153)
        at org.kohsuke.stapler.export.Model.writeNestedObjectTo(Model.java:153)
        at org.kohsuke.stapler.export.Model.writeNestedObjectTo(Model.java:153)
        at org.kohsuke.stapler.export.Model.writeNestedObjectTo(Model.java:153)
        at org.kohsuke.stapler.export.Model.writeNestedObjectTo(Model.java:153)
        at org.kohsuke.stapler.export.Model.writeTo(Model.java:129)
        at org.kohsuke.stapler.ResponseImpl.serveExposedBean(ResponseImpl.java:176)
        at hudson.model.Api.doJson(Api.java:181)



"pool-6-thread-250":
        at hudson.model.AbstractProject.getActions(AbstractProject.java:898)
->      - waiting to lock <0x00002aaab7099118> (a hudson.maven.MavenModuleSet)
        at hudson.model.Actionable.getActions(Actionable.java:75)
        at hudson.plugins.jobConfigHistory.JobConfigHistoryActionFactory.createFor(JobConfigHistoryActionFactory.java:30)
        at hudson.model.AbstractProject.updateTransientActions(AbstractProject.java:549)
        at hudson.maven.AbstractMavenProject.updateTransientActions(AbstractMavenProject.java:48)
->      - locked <0x00002aab02808368> (a java.util.Vector)
        at hudson.maven.MavenModuleSet.updateTransientActions(MavenModuleSet.java:199)
        at hudson.maven.MavenModuleSetBuild.notifyModuleBuild(MavenModuleSetBuild.java:347)
        - locked <0x00002aab0103d910> (a hudson.maven.MavenModuleSetBuild)
        at hudson.maven.MavenBuild$ProxyImpl2.end(MavenBuild.java:437)
        at sun.reflect.GeneratedMethodAccessor368.invoke(Unknown Source)

git-svn-id: https://hudson.dev.java.net/svn/hudson/trunk/hudson/main@35610 71c3de6d-444a-0410-be80-ed276b4c234a

core/src/main/java/hudson/matrix/MatrixProject.java

@@ -28,6 +28,7 @@ import hudson.Extension;
 import hudson.Util;
 import hudson.XmlFile;
 import hudson.model.AbstractProject;
+import hudson.model.Action;
 import hudson.model.BuildableItemWithBuildWrappers;
 import hudson.model.DependencyGraph;
 import hudson.model.Descriptor;
@@ -246,19 +247,19 @@ public class MatrixProject extends AbstractProject<MatrixProject,MatrixBuild> im
     }
     
     @Override
-    protected void updateTransientActions() {
-        synchronized(transientActions) {
-            super.updateTransientActions();
-
-            for (BuildStep step : builders)
-                transientActions.addAll(step.getProjectActions(this));
-            for (BuildStep step : publishers)
-                transientActions.addAll(step.getProjectActions(this));
-            for (BuildWrapper step : buildWrappers)
-                transientActions.addAll(step.getProjectActions(this));
-            for (Trigger trigger : triggers)
-                transientActions.addAll(trigger.getProjectActions());
-        }
+    protected List<Action> createTransientActions() {
+        List<Action> r = super.createTransientActions();
+
+        for (BuildStep step : builders)
+            r.addAll(step.getProjectActions(this));
+        for (BuildStep step : publishers)
+            r.addAll(step.getProjectActions(this));
+        for (BuildWrapper step : buildWrappers)
+            r.addAll(step.getProjectActions(this));
+        for (Trigger trigger : triggers)
+            r.addAll(trigger.getProjectActions());
+
+        return r;
     }
 
     /**

core/src/main/java/hudson/model/AbstractProject.java

@@ -539,7 +539,18 @@ public abstract class AbstractProject<P extends AbstractProject<P,R>,R extends A
             return super.getIconColor();
     }
 
+    /**
+     * effectively deprecated. Since using updateTransientActions correctly
+     * under concurrent environment requires a lock that can too easily cause deadlocks.
+     *
+     * <p>
+     * Override {@link #createTransientActions()} instead.
+     */
     protected void updateTransientActions() {
+        transientActions = createTransientActions();
+    }
+
+    protected List<Action> createTransientActions() {
         Vector<Action> ta = new Vector<Action>();
 
         for (JobProperty<? super P> p : properties)
@@ -547,8 +558,7 @@ public abstract class AbstractProject<P extends AbstractProject<P,R>,R extends A
 
         for (TransientProjectActionFactory tpaf : TransientProjectActionFactory.all())
             ta.addAll(Util.fixNull(tpaf.createFor(this))); // be defensive against null
-
-        transientActions = ta;
+        return ta;
     }
 
     /**

core/src/main/java/hudson/model/Project.java

@@ -199,19 +199,19 @@ public abstract class Project<P extends Project<P,B>,B extends Build<P,B>>
     }
 
     @Override
-    protected synchronized void updateTransientActions() {
-        synchronized(transientActions) {
-            super.updateTransientActions();
-
-            for (BuildStep step : getBuildersList())
-                transientActions.addAll(step.getProjectActions(this));
-            for (BuildStep step : getPublishersList())
-                transientActions.addAll(step.getProjectActions(this));
-            for (BuildWrapper step : getBuildWrappers().values())
-                transientActions.addAll(step.getProjectActions(this));
-            for (Trigger trigger : getTriggers().values())
-                transientActions.addAll(trigger.getProjectActions());
-        }
+    protected List<Action> createTransientActions() {
+        List<Action> r = super.createTransientActions();
+
+        for (BuildStep step : getBuildersList())
+            r.addAll(step.getProjectActions(this));
+        for (BuildStep step : getPublishersList())
+            r.addAll(step.getProjectActions(this));
+        for (BuildWrapper step : getBuildWrappers().values())
+            r.addAll(step.getProjectActions(this));
+        for (Trigger trigger : getTriggers().values())
+            r.addAll(trigger.getProjectActions());
+
+        return r;
     }
 
     /**

maven-plugin/src/main/java/hudson/maven/AbstractMavenProject.java

@@ -25,11 +25,13 @@ package hudson.maven;
 
 import hudson.model.AbstractBuild;
 import hudson.model.AbstractProject;
+import hudson.model.Action;
 import hudson.model.ItemGroup;
 import hudson.tasks.Maven.ProjectWithMaven;
 import hudson.triggers.Trigger;
 
 import java.util.HashSet;
+import java.util.List;
 import java.util.Set;
 
 /**
@@ -43,23 +45,27 @@ public abstract class AbstractMavenProject<P extends AbstractProject<P,R>,R exte
         super(parent, name);
     }
 
-    protected void updateTransientActions() {
-        synchronized(transientActions) {
-            super.updateTransientActions();
+    protected List<Action> createTransientActions() {
+        List<Action> r = super.createTransientActions();
 
-            // if we just pick up the project actions from the last build,
-            // and if the last build failed very early, then the reports that
-            // kick in later (like test results) won't be displayed.
-            // so pick up last successful build, too.
-            Set<Class> added = new HashSet<Class>();
-            addTransientActionsFromBuild(getLastBuild(),added);
-            addTransientActionsFromBuild(getLastSuccessfulBuild(),added);
+        // if we just pick up the project actions from the last build,
+        // and if the last build failed very early, then the reports that
+        // kick in later (like test results) won't be displayed.
+        // so pick up last successful build, too.
+        Set<Class> added = new HashSet<Class>();
+        addTransientActionsFromBuild(getLastBuild(),r,added);
+        addTransientActionsFromBuild(getLastSuccessfulBuild(),r,added);
 
-            for (Trigger trigger : triggers)
-                transientActions.addAll(trigger.getProjectActions());
-        }
+        for (Trigger<?> trigger : triggers)
+            r.addAll(trigger.getProjectActions());
+
+        return r;
     }
-    
-    protected abstract void addTransientActionsFromBuild(R lastBuild, Set<Class> added);
+
+    /**
+     * @param collection
+     *      Add the transient actions to this collection.
+     */
+    protected abstract void addTransientActionsFromBuild(R lastBuild, List<Action> collection, Set<Class> added);
     
 }

maven-plugin/src/main/java/hudson/maven/MavenModule.java

@@ -28,6 +28,7 @@ import hudson.Util;
 import hudson.Functions;
 import hudson.maven.reporters.MavenMailer;
 import hudson.model.AbstractProject;
+import hudson.model.Action;
 import hudson.model.DependencyGraph;
 import hudson.model.Descriptor;
 import hudson.model.Descriptor.FormException;
@@ -370,6 +371,11 @@ public final class MavenModule extends AbstractMavenProject<MavenModule,MavenBui
         return true;
     }
 
+    @Override // to make this accessible to MavenModuleSet
+    protected void updateTransientActions() {
+        super.updateTransientActions();
+    }
+
     protected void buildDependencyGraph(DependencyGraph graph) {
         if(isDisabled() || getParent().ignoreUpstremChanges())        return;
 
@@ -410,7 +416,7 @@ public final class MavenModule extends AbstractMavenProject<MavenModule,MavenBui
     }
 
     @Override
-    protected void addTransientActionsFromBuild(MavenBuild build, Set<Class> added) {
+    protected void addTransientActionsFromBuild(MavenBuild build, List<Action> collection, Set<Class> added) {
         if(build==null)    return;
         List<MavenProjectActionBuilder> list = build.projectActionReporters;
         if(list==null)   return;
@@ -418,7 +424,7 @@ public final class MavenModule extends AbstractMavenProject<MavenModule,MavenBui
         for (MavenProjectActionBuilder step : list) {
             if(!added.add(step.getClass()))     continue;   // already added
             try {
-                transientActions.addAll(step.getProjectActions(this));
+                collection.addAll(step.getProjectActions(this));
             } catch (Exception e) {
                 LOGGER.log(Level.WARNING, "Failed to getProjectAction from " + step
                            + ". Report issue to plugin developers.", e);

maven-plugin/src/main/java/hudson/maven/MavenModuleSet.java

@@ -195,8 +195,9 @@ public final class MavenModuleSet extends AbstractMavenProject<MavenModuleSet,Ma
         return getItem(name);
     }
 
-    protected void updateTransientActions() {
-        super.updateTransientActions();
+    protected List<Action> createTransientActions() {
+        List<Action> r = super.createTransientActions();
+
         // Fix for ISSUE-1149
         for (MavenModule module: modules.values()) {
             module.updateTransientActions();
@@ -204,20 +205,22 @@ public final class MavenModuleSet extends AbstractMavenProject<MavenModuleSet,Ma
         
         if(publishers!=null)    // this method can be loaded from within the onLoad method, where this might be null
             for (BuildStep step : publishers)
-                transientActions.addAll(step.getProjectActions(this));
+                r.addAll(step.getProjectActions(this));
 
         if (buildWrappers!=null)
 	        for (BuildWrapper step : buildWrappers)
-                transientActions.addAll(step.getProjectActions(this));
+                r.addAll(step.getProjectActions(this));
+
+        return r;
     }
 
-    protected void addTransientActionsFromBuild(MavenModuleSetBuild build, Set<Class> added) {
+    protected void addTransientActionsFromBuild(MavenModuleSetBuild build, List<Action> collection, Set<Class> added) {
         if(build==null)    return;
 
         for (Action a : build.getActions())
             if(a instanceof MavenAggregatedReport)
                 if(added.add(a.getClass()))
-                    transientActions.add(((MavenAggregatedReport)a).getProjectAction(this));
+                    collection.add(((MavenAggregatedReport)a).getProjectAction(this));
 
         List<MavenReporter> list = build.projectActionReporters;
         if(list==null)   return;
@@ -226,7 +229,7 @@ public final class MavenModuleSet extends AbstractMavenProject<MavenModuleSet,Ma
             if(!added.add(step.getClass()))     continue;   // already added
             Action a = step.getAggregatedProjectAction(this);
             if(a!=null)
-                transientActions.add(a);
+                collection.add(a);
         }
     }