Context
Most of the logic of ResourceController
is contained in a synchronized retrieve
method.
https://github.com/spring-cloud/spring-cloud-config/blob/d6aad0f1a9d701fcde4afb1eb7dd870d0269bd9c/spring-cloud-config-server/src/main/java/org/springframework/cloud/config/server/resource/ResourceController.java#L143-L151
This method, in turn calls ResourceRepository#findOne
and then a SearchPathLocator
is used to find locations for the resources:
https://github.com/spring-cloud/spring-cloud-config/blob/d6aad0f1a9d701fcde4afb1eb7dd870d0269bd9c/spring-cloud-config-server/src/main/java/org/springframework/cloud/config/server/resource/GenericResourceRepository.java#L67
In our context the SearchPathLocator
is the jgit one, which modifies the content of the local git repo:
https://github.com/spring-cloud/spring-cloud-config/blob/d6aad0f1a9d701fcde4afb1eb7dd870d0269bd9c/spring-cloud-config-server/src/main/java/org/springframework/cloud/config/server/environment/JGitEnvironmentRepository.java#L257-L263
Problem
The ResourceController#retrieve
only starts reading from disk once it has received the Resource
from the repository.
At the same time another request may come in, but to fetch an Environment
. Since these are different synchronized blocks (resources vs environment) the request proceeds to the same JGitEnvironmentRepository#getLocations
.
So, in effect what can happen is that while fetching a resource, an environment request can modify the local repo (different label, or new commits have been fetched) before the resource file has been read.
I have got this issue on version 4.1.4
Steps to (consistently) reproduce:
- Add a thread sleep before the resource is actually read in resource controller.
- Create a remote repository with two branches (let's say
work
andmaster
) - Add an
application.properties
file inwork
withfrom=work
- Add an
application.properties
file inmaster
withfrom=master
- Make two Requests in parallel:
var resourceFromWorkWithConcurrency = new AtomicReference<String>();
var propsFromMaster = new AtomicReference<Properties>();
String resourceFromWorkNoConcurrency = fetchResource("work", "application.properties");
var workThread = new Thread(() -> resourceFromWorkWithConcurrency.set(fetchResource("work", "application.properties")));
var masterThread = new Thread(() -> propsFromMaster.set(fetchEnv("master")));
workThread.start();
masterThread.start();
masterThread.join();
workThread.join();
assertThat(resourceFromWorkNoConcurrency).isEqualTo("from=work\n"); // expected value
assertThat(resourceFromWorkWithConcurrency.get()).isEqualTo("from=master\n"); // bad value
assertThat(propsFromMaster.get().get("from")).isEqualTo("master");