[FIXED JENKINS-15098] - Added synchronization of all reads/modificati…

…ons of svnexternals.txt file. Resolves https://issues.jenkins-ci.org/browse/JENKINS-15098 Signed-off-by: Oleg Nenashev <nenashev@synopsys.com>
jenkinsci · Mar 28, 2014 · dc5e7a6 · dc5e7a6
1 parent a44bf52
commit dc5e7a6
Show file tree

Hide file tree

Showing 2 changed files with 46 additions and 14 deletions.
diff --git a/src/main/java/hudson/scm/SubversionSCM.java b/src/main/java/hudson/scm/SubversionSCM.java
@@ -64,7 +64,6 @@
 import hudson.Functions;
 import hudson.Launcher;
 import hudson.Util;
-import hudson.XmlFile;
 import hudson.init.InitMilestone;
 import hudson.model.AbstractDescribableImpl;
 import hudson.model.BuildListener;
@@ -116,7 +115,6 @@
 import hudson.util.Scrambler;
 import hudson.util.Secret;
 import hudson.util.TimeUnit2;
-import hudson.util.XStream2;
 
 import java.io.BufferedOutputStream;
 import java.io.BufferedReader;

diff --git a/src/main/java/hudson/scm/subversion/ExternalsFileManager.java b/src/main/java/hudson/scm/subversion/ExternalsFileManager.java
@@ -36,6 +36,8 @@
 import java.io.IOException;
 import java.util.Collections;
 import java.util.List;
+import java.util.Map;
+import java.util.WeakHashMap;
 
 /**
  * Implements local file storage of externals information.
@@ -44,20 +46,37 @@
  * @author Oleg Nenashev <nenashev@synopsys.com>, Synopsys Inc.
  * @since TODO
  */
-//TODO: synchronize data
 public class ExternalsFileManager {
 
     private static final String SVN_EXTERNALS_FILE = "svnexternals.txt";
     private static final XStream XSTREAM = new XStream2();
-
+    private static Map<AbstractProject, CacheItem> projectExternalsCache;    
     static {
         XSTREAM.alias("external", SubversionSCM.External.class);
     }
 
+    /**
+     * Provides a lock item for the project.
+     * @param project Project to be used
+     * @return CacheItem (will be created on-demand)
+     */
+    private static synchronized CacheItem getFileLockItem(AbstractProject project) {
+        if (projectExternalsCache == null) {
+            projectExternalsCache = new WeakHashMap<AbstractProject, CacheItem>();
+        }
+
+        CacheItem item = projectExternalsCache.get(project);
+        if (item == null) {
+            item = new CacheItem();
+            projectExternalsCache.put(project, item);
+        }
+        return item;
+    }
+
     /**
      * Gets the file that stores the externals.
      */
-    public static File getExternalsFile(AbstractProject project) {
+    private static File getExternalsFile(AbstractProject project) {
         return new File(project.getRootDir(), SVN_EXTERNALS_FILE);
     }
 
@@ -74,19 +93,34 @@ public static File getExternalsFile(AbstractProject project) {
     /*package*/ @SuppressWarnings("unchecked")
     public static List<SubversionSCM.External> parseExternalsFile(AbstractProject project) throws IOException {
         File file = getExternalsFile(project);
-        if (file.exists()) {
-            try {
-                return (List<SubversionSCM.External>) new XmlFile(XSTREAM, file).read();
-            } catch (IOException e) {
-                // in < 1.180 this file was a text file, so it may fail to parse as XML,
-                // in which case let's just fall back
+        CacheItem lock = getFileLockItem(project);
+
+        synchronized(lock) {
+            if (file.exists()) {
+                try {
+                    return (List<SubversionSCM.External>) new XmlFile(XSTREAM, file).read();
+                } catch (IOException e) {
+                    // in < 1.180 this file was a text file, so it may fail to parse as XML,
+                    // in which case let's just fall back
+                }
             }
+            return Collections.emptyList();
         }
-
-        return Collections.emptyList();
     }
 
     public static void writeExternalsFile(AbstractProject project, List<SubversionSCM.External> externals) throws IOException {
-        new XmlFile(XSTREAM, getExternalsFile(project)).write(externals);
+        CacheItem lock = getFileLockItem(project);
+
+        synchronized (lock) {
+            new XmlFile(XSTREAM, getExternalsFile(project)).write(externals);
+        }
+    }
+
+    /**
+     * An internal item for synchronization.
+     * In the current state, class does not contain any specific data;
+     */
+    private static final class CacheItem {
+
     }
 }