Cleanup: attempt to fix race condition.

The latestCommitFuture is global, but was not protected against
concurrent modifications.

Code changes:

* Make sure that adding the commit to the queue and queueing the new
  job on the executor happen synchronized.
* Eliminate the global latestCommitFuture altogether. It's good enough
  to wait for the current transactions' commit future.

Author: tomaswolf

src/main/java/hudson/plugins/scm_sync_configuration/ScmSyncConfigurationBusiness.java

@@ -47,7 +47,7 @@ public class ScmSyncConfigurationBusiness {
     /*package*/ final ExecutorService writer = Executors.newFixedThreadPool(1, new DaemonThreadFactory());
 
     //  TODO: Refactor this into the plugin object ???
-    private List<Commit> commitsQueue = Collections.synchronizedList(new ArrayList<Commit>());
+    private List<Commit> commitsQueue = new ArrayList<Commit>();
 
     public ScmSyncConfigurationBusiness(){
     }
@@ -139,21 +139,26 @@ public Future<Void> queueChangeSet(final ScmContext scmContext, ChangeSet change
 
         Commit commit = new Commit(changeset, user, userMessage, scmContext);
         LOGGER.finest("Queuing commit "+commit.toString()+" to SCM ...");
-        commitsQueue.add(commit);
-
-        return writer.submit(new Callable<Void>() {
-            public Void call() throws Exception {
-                processCommitsQueue();
-                return null;
-            }
-        });
+        synchronized(commitsQueue) {
+	        commitsQueue.add(commit);
+	
+	        return writer.submit(new Callable<Void>() {
+	            public Void call() throws Exception {
+	                processCommitsQueue();
+	                return null;
+	            }
+	        });
+        }
     }
 
     private void processCommitsQueue() {
         File scmRoot = new File(getCheckoutScmDirectoryAbsolutePath());
 
         // Copying shared commitQueue in order to allow conccurrent modification
-        List<Commit> currentCommitQueue = new ArrayList<Commit>(commitsQueue);
+        List<Commit> currentCommitQueue;
+        synchronized (commitsQueue) {
+        	currentCommitQueue = new ArrayList<Commit>(commitsQueue);
+        }
         List<Commit> checkedInCommits = new ArrayList<Commit>();
 
         try {
@@ -227,7 +232,9 @@ private void processCommitsQueue() {
             signal(e.getMessage(), false);
         } finally {
             // We should remove every checkedInCommits
-            commitsQueue.removeAll(checkedInCommits);
+        	synchronized (commitsQueue) {
+        		commitsQueue.removeAll(checkedInCommits);
+        	}
         }
     }
 

src/main/java/hudson/plugins/scm_sync_configuration/ScmSyncConfigurationPlugin.java

@@ -105,8 +105,6 @@ public static interface AtomicTransactionFactory {
      */
     private transient ThreadLocal<ScmTransaction> transaction = new ThreadLocal<ScmTransaction>();
 
-    private transient Future<Void> latestCommitFuture;
-
 	private String scmRepositoryUrl;
 	private SCM scm;
 	private boolean noUserCommitMessage;
@@ -434,8 +432,7 @@ public void startThreadedTransaction(){
     public Future<Void> commitChangeset(ChangeSet changeset){
         try {
             if(!changeset.isEmpty()){
-                latestCommitFuture = this.business.queueChangeSet(createScmContext(), changeset, getCurrentUser(), ScmSyncConfigurationDataProvider.retrieveComment(false));
-                return latestCommitFuture;
+                return this.business.queueChangeSet(createScmContext(), changeset, getCurrentUser(), ScmSyncConfigurationDataProvider.retrieveComment(false));
             } else {
                 return null;
             }
@@ -462,10 +459,6 @@ protected void setTransaction(ScmTransaction transactionToRegister){
     public boolean currentUserCannotPurgeFailLogs() {
         return !business.canCurrentUserPurgeFailLogs();
     }
-
-    public Future<Void> getLatestCommitFuture() {
-        return latestCommitFuture;
-    }
 
     private static final Pattern STARTS_WITH_DRIVE_LETTER = Pattern.compile("^[a-zA-Z]:");
 

src/main/java/hudson/plugins/scm_sync_configuration/transactions/ScmTransaction.java

@@ -1,6 +1,7 @@
 package hudson.plugins.scm_sync_configuration.transactions;
 
 import java.io.File;
+import java.util.concurrent.Future;
 
 import hudson.plugins.scm_sync_configuration.JenkinsFilesHelper;
 import hudson.plugins.scm_sync_configuration.ScmSyncConfigurationPlugin;
@@ -30,12 +31,11 @@ public void defineCommitMessage(WeightedMessage weightedMessage){
     }
 
     public void commit(){
-        ScmSyncConfigurationPlugin.getInstance().commitChangeset(changeset);
-        if(synchronousCommit){
-            // Synchronous transactions should wait for latest commit future to be fully processed
-            // before going further
+        Future<Void> future = ScmSyncConfigurationPlugin.getInstance().commitChangeset(changeset);
+        if (synchronousCommit && future != null) {
+            // Synchronous transactions should wait for the future to be fully processed
             try {
-               ScmSyncConfigurationPlugin.getInstance().getLatestCommitFuture().get();
+               future.get();
             } catch (Exception e) {
                throw new RuntimeException(e);
             }