Skip to content

Race condition between environment and resources repository when using jgit repository #2681

@dpozinen

Description

@dpozinen

Context

Most of the logic of ResourceController is contained in a synchronized retrieve method.

name = Environment.normalize(name);
label = Environment.normalize(label);
Resource resource = this.resourceRepository.findOne(name, profile, label, path);
if (checkNotModified(request, resource)) {
// Content was not modified. Just return.
return null;
}
// ensure InputStream will be closed to prevent file locks on Windows
try (InputStream is = resource.getInputStream()) {

This method, in turn calls ResourceRepository#findOne and then a SearchPathLocator is used to find locations for the resources:

String[] locations = this.service.getLocations(application, profile, label).getLocations();

In our context the SearchPathLocator is the jgit one, which modifies the content of the local git repo:

if (label == null) {
label = this.defaultLabel;
}
String version;
try {
version = refresh(label);
}

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:

  1. Add a thread sleep before the resource is actually read in resource controller.
  2. Create a remote repository with two branches (let's say work and master)
  3. Add an application.properties file in work with from=work
  4. Add an application.properties file in master with from=master
  5. 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");

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type

    Projects

    Milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions