Synchronize access to code that deletes basedir on startup

When the first request comes in there is a potential race while the
basedir is prepared (if another request comes in at that crucial
time they collide). This change just synchronizes the method where
the basedir is prepared (it's a once only operation).

Fixes gh-187

spring-cloud-config-server/src/main/java/org/springframework/cloud/config/server/JGitEnvironmentRepository.java

@@ -71,8 +71,7 @@ public class JGitEnvironmentRepository extends AbstractScmEnvironmentRepository
 
 	private boolean cloneOnStart = false;
 
-	private JGitEnvironmentRepository.JGitFactory gitFactory =
-			new JGitEnvironmentRepository.JGitFactory();
+	private JGitEnvironmentRepository.JGitFactory gitFactory = new JGitEnvironmentRepository.JGitFactory();
 
 	public JGitEnvironmentRepository(ConfigurableEnvironment environment) {
 		super(environment);
@@ -157,7 +156,8 @@ public class JGitEnvironmentRepository extends AbstractScmEnvironmentRepository
 
 	private synchronized Environment loadEnvironment(Git git, String application,
 			String profile, String label) throws GitAPIException {
-		NativeEnvironmentRepository environment = new NativeEnvironmentRepository(getEnvironment());
+		NativeEnvironmentRepository environment = new NativeEnvironmentRepository(
+				getEnvironment());
 		git.getRepository().getConfig().setString("branch", label, "merge", label);
 		Ref ref = checkout(git, label);
 		if (shouldPull(git, ref)) {
@@ -223,7 +223,10 @@ public class JGitEnvironmentRepository extends AbstractScmEnvironmentRepository
 		}
 	}
 
-	private Git copyRepository() throws IOException, GitAPIException {
+	// Synchronize here so that multiple requests don't all try and delete the base dir
+	// together (this is a once only operation, so it only holds things up on the first
+	// request).
+	private synchronized Git copyRepository() throws IOException, GitAPIException {
 		deleteBaseDirIfExists();
 		getBasedir().mkdirs();
 		Assert.state(getBasedir().exists(), "Could not create basedir: " + getBasedir());
@@ -304,7 +307,7 @@ public class JGitEnvironmentRepository extends AbstractScmEnvironmentRepository
 				getPassword()));
 	}
 
-	private void setTimeout(TransportCommand<?,?> pull) {
+	private void setTimeout(TransportCommand<?, ?> pull) {
 		pull.setTimeout(this.timeout);
 	}
 
@@ -339,8 +342,7 @@ public class JGitEnvironmentRepository extends AbstractScmEnvironmentRepository
 
 	/**
 	 * Wraps the static method calls to {@link org.eclipse.jgit.api.Git} and
-	 * {@link org.eclipse.jgit.api.CloneCommand} allowing for easier unit
-	 * testing.
+	 * {@link org.eclipse.jgit.api.CloneCommand} allowing for easier unit testing.
 	 */
 	static class JGitFactory {
 

spring-cloud-config-server/src/test/java/org/springframework/cloud/config/server/JGitEnvironmentRepositoryConcurrencyTests.java

@@ -0,0 +1,99 @@
+/*
+ * Copyright 2013-2015 the original author or authors.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.springframework.cloud.config.server;
+
+import static org.junit.Assert.assertArrayEquals;
+import static org.junit.Assert.assertEquals;
+
+import java.io.File;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+import java.util.concurrent.Future;
+
+import org.eclipse.jgit.util.FileUtils;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+import org.springframework.boot.autoconfigure.PropertyPlaceholderAutoConfiguration;
+import org.springframework.boot.builder.SpringApplicationBuilder;
+import org.springframework.cloud.config.environment.Environment;
+import org.springframework.cloud.config.server.config.EnvironmentRepositoryConfiguration;
+import org.springframework.context.ConfigurableApplicationContext;
+import org.springframework.context.annotation.Configuration;
+import org.springframework.context.annotation.Import;
+
+/**
+ * @author Dave Syer
+ * @author Roy Clarkson
+ */
+public class JGitEnvironmentRepositoryConcurrencyTests {
+
+	private ConfigurableApplicationContext context;
+
+	private File basedir = new File("target/config");
+
+	@Before
+	public void init() throws Exception {
+		if (this.basedir.exists()) {
+			FileUtils.delete(this.basedir, FileUtils.RECURSIVE);
+		}
+		ConfigServerTestUtils.deleteLocalRepo("config-copy");
+	}
+
+	@After
+	public void close() {
+		if (this.context != null) {
+			this.context.close();
+		}
+	}
+
+	@Test
+	public void vanilla() throws Exception {
+		String uri = ConfigServerTestUtils.prepareLocalRepo();
+		this.context = new SpringApplicationBuilder(TestConfiguration.class).web(false)
+				.properties("spring.cloud.config.server.git.uri:" + uri).run();
+		final EnvironmentRepository repository = this.context
+				.getBean(EnvironmentRepository.class);
+		ExecutorService threads = Executors.newFixedThreadPool(4);
+		List<Future<Boolean>> tasks = new ArrayList<Future<Boolean>>();
+		for (int i=0; i<30; i++) {
+			tasks.add(threads.submit(new Runnable() {
+				@Override
+				public void run() {
+					repository.findOne("bar", "staging", "master");
+				}
+			}, true));
+		}
+		for (Future<Boolean> future : tasks) {
+			future.get();
+		}
+		Environment environment = repository.findOne("bar", "staging", "master");
+		assertEquals(2, environment.getPropertySources().size());
+		assertEquals("bar", environment.getName());
+		assertArrayEquals(new String[] { "staging" }, environment.getProfiles());
+		assertEquals("master", environment.getLabel());
+	}
+
+	@Configuration
+	@Import({ PropertyPlaceholderAutoConfiguration.class,
+		EnvironmentRepositoryConfiguration.class })
+	protected static class TestConfiguration {
+	}
+
+}