Synchronize access to the build number.

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

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

@@ -156,7 +156,9 @@ public abstract class Job<JobT extends Job<JobT, RunT>, RunT extends Run<JobT, R
             // but old Hudson didn't do it, so if the file doesn't exist,
             // assume that nextBuildNumber was read from config.xml
             try {
-                this.nextBuildNumber = Integer.parseInt(f.readTrim());
+                synchronized (this) {
+                    this.nextBuildNumber = Integer.parseInt(f.readTrim());
+                }
             } catch (NumberFormatException e) {
                 throw new IOException2(f + " doesn't contain a number", e);
             }
@@ -176,7 +178,9 @@ public abstract class Job<JobT extends Job<JobT, RunT>, RunT extends Run<JobT, R
     @Override
     public void onCopiedFrom(Item src) {
         super.onCopiedFrom(src);
-        this.nextBuildNumber = 1; // reset the next build number
+        synchronized (this) {
+            this.nextBuildNumber = 1; // reset the next build number
+        }
     }
 
     @Override
@@ -193,11 +197,11 @@ public abstract class Job<JobT extends Job<JobT, RunT>, RunT extends Run<JobT, R
         super.performDelete();
     }
 
-    private TextFile getNextBuildNumberFile() {
+    /*package*/ TextFile getNextBuildNumberFile() {
         return new TextFile(new File(this.getRootDir(), "nextBuildNumber"));
     }
 
-    protected void saveNextBuildNumber() throws IOException {
+    protected synchronized void saveNextBuildNumber() throws IOException {
         if (nextBuildNumber == 0) { // #3361
             nextBuildNumber = 1;
         }
@@ -275,7 +279,7 @@ public abstract class Job<JobT extends Job<JobT, RunT>, RunT extends Run<JobT, R
      * 
      * @since 1.199 (before that, this method was package private.)
      */
-    public void updateNextBuildNumber(int next) throws IOException {
+    public synchronized void updateNextBuildNumber(int next) throws IOException {
         if (next > nextBuildNumber) {
             this.nextBuildNumber = next;
             saveNextBuildNumber();

test/src/test/java/hudson/model/JobTest.java

@@ -26,6 +26,9 @@ package hudson.model;
 import com.gargoylesoftware.htmlunit.WebAssert;
 import com.gargoylesoftware.htmlunit.html.HtmlPage;
 
+import hudson.util.TextFile;
+import java.io.IOException;
+import java.util.concurrent.CountDownLatch;
 import org.jvnet.hudson.test.HudsonTestCase;
 
 /**
@@ -41,7 +44,95 @@ public class JobTest extends HudsonTestCase {
         HtmlPage page = new WebClient().getPage(project);
         WebAssert.assertTextPresent(page, "NeedleInPage");
     }
-    
+
+    public void testBuildNumberSynchronization() throws Exception {
+        AbstractProject project = createFreeStyleProject();
+        CountDownLatch startLatch = new CountDownLatch(1);
+        CountDownLatch stopLatch = new CountDownLatch(2);
+        BuildNumberSyncTester test1 = new BuildNumberSyncTester(project, startLatch, stopLatch, true);
+        BuildNumberSyncTester test2 = new BuildNumberSyncTester(project, startLatch, stopLatch, false);
+        new Thread(test1).start();
+        new Thread(test2).start();
+
+        startLatch.countDown();
+        stopLatch.await();
+
+        assertTrue(test1.message, test2.passed);
+        assertTrue(test2.message, test2.passed);
+    }
+
+    public static class BuildNumberSyncTester implements Runnable {
+        private final AbstractProject p;
+        private final CountDownLatch start;
+        private final CountDownLatch stop;
+        private final boolean assign;
+
+        String message;
+        boolean passed;
+
+        BuildNumberSyncTester(AbstractProject p, CountDownLatch l1, CountDownLatch l2, boolean b) {
+            this.p = p;
+            this.start = l1;
+            this.stop = l2;
+            this.assign = b;
+            this.message = null;
+            this.passed = false;
+        }
+
+        public void run() {
+            try {
+                start.await();
+
+                for (int i = 0; i < 100; i++) {
+                    int buildNumber = -1, savedBuildNumber = -1;
+                    TextFile f;
+
+                    synchronized (p) {
+                        if (assign) {
+                            buildNumber = p.assignBuildNumber();
+                            f = p.getNextBuildNumberFile();
+                            if (f == null) {
+                                this.message = "Could not get build number file";
+                                this.passed = false;
+                                return;
+                            }
+                            savedBuildNumber = Integer.parseInt(f.readTrim());
+                            if (buildNumber != (savedBuildNumber-1)) {
+                                this.message = "Build numbers don't match (" + buildNumber + ", " + (savedBuildNumber-1) + ")";
+                                this.passed = false;
+                                return;
+                            }
+                        } else {
+                            buildNumber = p.getNextBuildNumber() + 100;
+                            p.updateNextBuildNumber(buildNumber);
+                            f = p.getNextBuildNumberFile();
+                            if (f == null) {
+                                this.message = "Could not get build number file";
+                                this.passed = false;
+                                return;
+                            }
+                            savedBuildNumber = Integer.parseInt(f.readTrim());
+                            if (buildNumber != savedBuildNumber) {
+                                this.message = "Build numbers don't match (" + buildNumber + ", " + savedBuildNumber + ")";
+                                this.passed = false;
+                                return;
+                            }
+                        }
+                    }
+                }
+
+                this.passed = true;
+            }
+            catch (InterruptedException e) {}
+            catch (IOException e) {
+                fail("Failed to assign build number");
+            }
+            finally {
+                stop.countDown();
+            }
+        }
+    }
+
     @SuppressWarnings("unchecked")
     public static class JobPropertyImpl extends JobProperty<Job<?,?>> {
         public static DescriptorImpl DESCRIPTOR = new DescriptorImpl();