Skip to content

Commit

Permalink
[Build] Fix Concurrency issue in buildparams access
Browse files Browse the repository at this point in the history
Also provide caching support for buildparams provider
  • Loading branch information
breskeby committed Nov 26, 2024
1 parent 5d9ad67 commit 7a8cd0d
Show file tree
Hide file tree
Showing 4 changed files with 139 additions and 9 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ private static void disableTransitiveDependenciesForSourceSet(Project project, S
public void configureCompile(Project project) {
project.getExtensions().getExtraProperties().set("compactProfile", "full");
JavaPluginExtension java = project.getExtensions().getByType(JavaPluginExtension.class);
if (buildParams.getJavaToolChainSpec().isPresent()) {
if (buildParams.getJavaToolChainSpec().getOrNull() != null) {
java.toolchain(buildParams.getJavaToolChainSpec().get());
}
java.setSourceCompatibility(buildParams.getMinimumRuntimeVersion());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import java.util.List;
import java.util.Random;
import java.util.concurrent.atomic.AtomicReference;
import java.util.function.Supplier;

public abstract class BuildParameterExtension {
private final Provider<Boolean> inFipsJvm;
Expand Down Expand Up @@ -56,19 +57,18 @@ public BuildParameterExtension(
JavaVersion gradleJavaVersion,
String gitRevision,
String gitOrigin,
ZonedDateTime buildDate,
String testSeed,
boolean isCi,
int defaultParallel,
final boolean isSnapshotBuild,
Provider<BwcVersions> bwcVersions
) {
this.inFipsJvm = providers.systemProperty("tests.fips.enabled").map(BuildParameterExtension::parseBoolean);
this.runtimeJavaHome = runtimeJavaHome;
this.javaToolChainSpec = javaToolChainSpec;
this.runtimeJavaVersion = runtimeJavaVersion;
this.runtimeJavaHome = cache(providers, runtimeJavaHome);
this.javaToolChainSpec = cache(providers, javaToolChainSpec);
this.runtimeJavaVersion = cache(providers, runtimeJavaVersion);
this.isRuntimeJavaHomeSet = isRuntimeJavaHomeSet;
this.runtimeJavaDetails = runtimeJavaDetails;
this.runtimeJavaDetails = cache(providers, runtimeJavaDetails);
this.javaVersions = javaVersions;
this.minimumCompilerVersion = minimumCompilerVersion;
this.minimumRuntimeVersion = minimumRuntimeVersion;
Expand All @@ -82,6 +82,12 @@ public BuildParameterExtension(
this.getGitOriginProperty().set(gitOrigin);
}

//This is a workaround for https://github.com/gradle/gradle/issues/25550
private <T> Provider<T> cache(ProviderFactory providerFactory, Provider<T> incomingProvider) {
SingleObjectCache<T> cache = new SingleObjectCache<>();
return providerFactory.provider(() -> cache.computeIfAbsent(() -> incomingProvider.getOrNull()));
}

private static boolean parseBoolean(String s) {
if (s == null) {
return false;
Expand Down Expand Up @@ -184,4 +190,21 @@ public Random getRandom() {
public Boolean isGraalVmRuntime() {
return runtimeJavaDetails.get().toLowerCase().contains("graalvm");
}

private static class SingleObjectCache<T> {
private T instance;

public T computeIfAbsent(Supplier<T> supplier) {
synchronized (this) {
if (instance == null) {
instance = supplier.get();
}
return instance;
}
}

public T get() {
return instance;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -47,8 +47,6 @@
import java.io.InputStreamReader;
import java.io.UncheckedIOException;
import java.nio.file.Files;
import java.time.ZoneOffset;
import java.time.ZonedDateTime;
import java.util.List;
import java.util.Locale;
import java.util.Random;
Expand Down Expand Up @@ -120,6 +118,7 @@ public void apply(Project project) {
.create(
"buildParams",
BuildParameterExtension.class,
providers,
actualRuntimeJavaHome,
resolveToolchainSpecFromEnv(),
actualRuntimeJavaHome.map(
Expand All @@ -139,7 +138,6 @@ public void apply(Project project) {
Jvm.current().getJavaVersion(),
gitInfo.getRevision(),
gitInfo.getOrigin(),
ZonedDateTime.now(ZoneOffset.UTC),
getTestSeed(),
System.getenv("JENKINS_URL") != null || System.getenv("BUILDKITE_BUILD_URL") != null || System.getProperty("isCI") != null,
ParallelDetector.findDefaultParallel(project),
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,109 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the "Elastic License
* 2.0", the "GNU Affero General Public License v3.0 only", and the "Server Side
* Public License v 1"; you may not use this file except in compliance with, at
* your election, the "Elastic License 2.0", the "GNU Affero General Public
* License v3.0 only", or the "Server Side Public License, v 1".
*/

package org.elasticsearch.gradle.internal.info

import spock.lang.Specification

import org.elasticsearch.gradle.internal.BwcVersions
import org.gradle.api.JavaVersion
import org.gradle.api.Project
import org.gradle.api.provider.Provider
import org.gradle.api.provider.ProviderFactory
import org.gradle.testfixtures.ProjectBuilder
import org.junit.Assert

import java.util.concurrent.CountDownLatch
import java.util.concurrent.Executors
import java.util.concurrent.Future
import java.util.concurrent.TimeUnit
import java.util.concurrent.atomic.AtomicInteger

import static org.junit.Assert.fail

class BuildParameterExtensionSpec extends Specification {

ProjectBuilder projectBuilder = new ProjectBuilder()

def "#getterName is cached anc concurrently accessible"() {
given:
def project = projectBuilder.build()
def providers = project.getProviders();
def buildParams = extension(project, providers)
int numberOfThreads = 10;
when:
var service = Executors.newFixedThreadPool(10)
var latch = new CountDownLatch(numberOfThreads)
def testedProvider = buildParams."$getterName"()
def futures = (1..numberOfThreads).collect {
service.submit(
() -> {
try {
testedProvider.get()
} catch (AssertionError e) {
latch.countDown()
Assert.fail("Accessing cached provider more than once")
}
latch.countDown()
}
)
}
latch.await(10, TimeUnit.SECONDS)

then:
futures.size() == numberOfThreads
futures.every { it.state() == Future.State.SUCCESS }

where:
getterName << [
"getRuntimeJavaHome",
"getJavaToolChainSpec",
"getRuntimeJavaDetails",
"getRuntimeJavaVersion"
]

}

private BuildParameterExtension extension(Project project, ProviderFactory providers) {
return project.getExtensions().create(
"buildParameters", BuildParameterExtension.class,
providers,
providerMock(),
providerMock(),
providerMock(),
true,
providerMock(),
[
Mock(JavaHome),
Mock(JavaHome),
],
JavaVersion.VERSION_11,
JavaVersion.VERSION_11,
JavaVersion.VERSION_11,
"gitRevision",
"gitOrigin",
"testSeed",
false,
5,
true,
// cannot use Mock here because of the way the provider is used by gradle internal property api
providers.provider { Mock(BwcVersions) }
)
}

private Provider providerMock() {
Provider provider = Mock(Provider)
AtomicInteger counter = new AtomicInteger(0)
provider.getOrNull() >> {
println "accessing provider"
return counter.get() == 1 ? fail("Accessing cached provider more than once") : counter.incrementAndGet()
}
provider
}
}

0 comments on commit 7a8cd0d

Please sign in to comment.