[FIXED JENKINS-22001] Simpler fix of JENKINS-19446 that does not intr…

…oduce asynchronous behavior.
jenkinsci · Feb 28, 2014 · 00d2717 · 00d2717
1 parent 3aa0dd5
commit 00d2717
Show file tree

Hide file tree

Showing 3 changed files with 14 additions and 22 deletions.
diff --git a/core/src/main/java/hudson/model/AbstractItem.java b/core/src/main/java/hudson/model/AbstractItem.java
@@ -43,7 +43,6 @@
 import hudson.util.AtomicFileWriter;
 import hudson.util.IOUtils;
 import jenkins.model.Jenkins;
-import jenkins.model.Jenkins.MasterComputer;
 import org.apache.tools.ant.taskdefs.Copy;
 import org.apache.tools.ant.types.FileSet;
 import org.kohsuke.stapler.WebMethod;
@@ -508,27 +507,21 @@ public void delete( StaplerRequest req, StaplerResponse rsp ) throws IOException
      * Any exception indicates the deletion has failed, but {@link AbortException} would prevent the caller
      * from showing the stack trace. This
      */
-    public synchronized void delete() throws IOException, InterruptedException {
+    public void delete() throws IOException, InterruptedException {
         checkPermission(DELETE);
-        performDelete();
-
-        // defer the notification to avoid the lock ordering problem. See JENKINS-19446.
-        MasterComputer.threadPoolForRemoting.submit(new java.util.concurrent.Callable<Void>() {
-            @Override
-            public Void call() throws Exception {
-                invokeOnDeleted();
-                Jenkins.getInstance().rebuildDependencyGraphAsync();
-                return null;
-            }
+        synchronized (this) { // could just make performDelete synchronized but overriders might not honor that
+            performDelete();
+        } // JENKINS-19446: leave synch block, but JENKINS-22001: still notify synchronously
+        invokeOnDeleted();
+        Jenkins.getInstance().rebuildDependencyGraphAsync();
+    }
 
-            /**
-             * A pointless function to work around what appears to be a HotSpot problem. See JENKINS-5756 and bug 6933067
-             * on BugParade for more details.
-             */
-            private void invokeOnDeleted() throws IOException {
-                getParent().onDeleted(AbstractItem.this);
-            }
-        });
+    /**
+     * A pointless function to work around what appears to be a HotSpot problem. See JENKINS-5756 and bug 6933067
+     * on BugParade for more details.
+     */
+    private void invokeOnDeleted() throws IOException {
+        getParent().onDeleted(AbstractItem.this);
     }
 
     /**

diff --git a/core/src/main/java/hudson/model/Job.java b/core/src/main/java/hudson/model/Job.java
@@ -617,7 +617,7 @@ public void renameTo(String newName) throws IOException {
         }
     }
 
-    @Override public synchronized void delete() throws IOException, InterruptedException {
+    @Override public void delete() throws IOException, InterruptedException {
         super.delete();
         Util.deleteRecursive(getBuildDir());
     }

diff --git a/test/src/test/java/hudson/model/ListViewTest.java b/test/src/test/java/hudson/model/ListViewTest.java
@@ -128,7 +128,6 @@ private void checkLinkFromItemExistsAndIsValid(Item item, ItemGroup ig, Item top
         MockFolder stuff = top.createProject(MockFolder.class, "stuff");
         Items.move(p1, stuff);
         p3.delete();
-        Thread.sleep(500); // TODO working around crappy JENKINS-19446 fix
         top.createProject(FreeStyleProject.class, "p3");
         assertEquals(new HashSet<TopLevelItem>(Arrays.asList(p1, p2)), new HashSet<TopLevelItem>(v.getItems()));
         top.renameTo("upper");