[JENKINS-15156] Refactored fix.

jenkinsci · Feb 6, 2013 · fe95fc5 · fe95fc5
1 parent 8f62901
commit fe95fc5
Show file tree

Hide file tree

Showing 4 changed files with 33 additions and 52 deletions.
diff --git a/core/src/main/java/hudson/model/AbstractProject.java b/core/src/main/java/hudson/model/AbstractProject.java
@@ -271,23 +271,15 @@ public void onCreatedFromScratch() {
         super.onCreatedFromScratch();
         // solicit initial contributions, especially from TransientProjectActionFactory
         updateTransientActions();
-        assert builds==null;
-        builds = new RunMap<R>(getBuildDir(), new Constructor<R>() {
-            public R create(File dir) throws IOException {
-        	return loadBuild(dir);
-            }
-        });
+        assert builds == null;
+        builds = createBuildRunMap();
     }
 
     @Override
     public void onLoad(ItemGroup<? extends Item> parent, String name) throws IOException {
         super.onLoad(parent, name);
 
-        RunMap<R> builds = new RunMap<R>(getBuildDir(), new Constructor<R>() {
-            public R create(File dir) throws IOException {
-                return loadBuild(dir);
-            }
-        });
+        RunMap<R> builds = createBuildRunMap();
         if (this.builds!=null) {
             // if we are reloading, keep all those that are still building intact
             for (R r : this.builds.getLoadedBuilds().values()) {
@@ -306,6 +298,14 @@ public R create(File dir) throws IOException {
         updateTransientActions();
     }
 
+    private RunMap<R> createBuildRunMap() {
+        return new RunMap<R>(getBuildDir(), new Constructor<R>() {
+            public R create(File dir) throws IOException {
+                return loadBuild(dir);
+            }
+        });
+    }
+
     private synchronized List<Trigger<?>> triggers() {
         if (triggers == null) {
             triggers = new Vector<Trigger<?>>();

diff --git a/core/src/main/java/jenkins/model/lazy/AbstractLazyLoadRunMap.java b/core/src/main/java/jenkins/model/lazy/AbstractLazyLoadRunMap.java
@@ -203,7 +203,7 @@ protected void initBaseDir(File dir) {
     private void loadIdOnDisk() {
         String[] buildDirs = dir.list(createDirectoryFilter());
         if (buildDirs==null) {
-            // the job may just have been created
+            // the job may have just been created
             buildDirs=EMPTY_STRING_ARRAY;
         }
         // wrap into ArrayList to enable mutation

diff --git a/test/src/main/java/org/jvnet/hudson/test/MemoryAssert.java b/test/src/main/java/org/jvnet/hudson/test/MemoryAssert.java
@@ -27,6 +27,7 @@
 import java.lang.ref.WeakReference;
 import java.util.ArrayList;
 import java.util.Collections;
+import java.util.HashSet;
 import java.util.List;
 import java.util.Set;
 import java.util.concurrent.Callable;
@@ -128,4 +129,20 @@ public static List<HistogramElement> increasedMemory(Callable<Void> callable, Fi
         return elements;
     }
 
+    /**
+     * Forces GC by causing an OOM and then verifies the given {@link WeakReference} has been garbage collected.
+     * @param reference object used to verify garbage collection.
+     */
+    public static void assertGC(WeakReference<?> reference) {
+        Set<Object[]> objects = new HashSet<Object[]>();
+        while (true) {
+            try {
+                objects.add(new Object[1024]);
+            } catch (OutOfMemoryError ignore) {
+                break;
+            }
+        }
+        assertTrue(reference.get() == null);
+    }
+
 }
diff --git a/test/src/test/java/hudson/model/AbstractProjectTest.java b/test/src/test/java/hudson/model/AbstractProjectTest.java
@@ -41,15 +41,14 @@
 import java.io.IOException;
 import org.jvnet.hudson.test.HudsonTestCase;
 import org.jvnet.hudson.test.Bug;
+import org.jvnet.hudson.test.MemoryAssert;
 import org.jvnet.hudson.test.recipes.PresetData;
 import org.jvnet.hudson.test.recipes.PresetData.DataSet;
 
 import java.io.File;
 import java.util.concurrent.Future;
 import org.apache.commons.io.FileUtils;
-import java.lang.ref.Reference;
 import java.lang.ref.WeakReference;
-import java.util.ArrayList;
 
 /**
  * @author Kohsuke Kawaguchi
@@ -284,46 +283,11 @@ public void testSymlinkForPostBuildFailure() throws Exception {
         assertSymlinkForBuild(lastStable, 1);
     }
 
-    // Force garbage collection of ref's referent. Taken from NetBeans code.
-    public static void forceGc(final Reference<?> ref) {
-        ArrayList<byte[]> alloc = new ArrayList<byte[]>();
-        int size = 100000;
-        System.out.print("forcing garbage collection...");
-        for (int i = 0; i < 50; i++) {
-            if (ref.get() == null) {
-                System.out.println("succeeded");
-                return;
-            }
-            try {
-                System.gc();
-                System.runFinalization();
-            } catch (OutOfMemoryError error) {
-                // OK
-            }
-            try {
-                alloc.add(new byte[size]);
-                size = (int)(((double)size) * 1.3);
-            } catch (OutOfMemoryError error) {
-                size = size / 2;
-            }
-            try {
-                if (i % 3 == 0) {
-                    Thread.sleep(321);
-                }
-            } catch (InterruptedException t) {
-                // ignore
-            }
-        }
-        System.out.println("failed");
-    }
-
     @Bug(15156)
-    public void testGetBuildAfterGc() throws Exception {
+    public void testGetBuildAfterGC() throws Exception {
         FreeStyleProject job = createFreeStyleProject();
-        job.scheduleBuild2(0, new Cause.UserCause()).get();
-        // If this fails to garbage-collect, the test run will not have any value, but it will
-        // at least not fail.
-        forceGc(new WeakReference(job.getLastBuild()));
+        job.scheduleBuild2(0, new Cause.UserIdCause()).get();
+        MemoryAssert.assertGC(new WeakReference(job.getLastBuild()));
         assertTrue(job.getLastBuild() != null);
     }
 }