Skip to content

Commit

Permalink
Remove detection of JDK-8274349
Browse files Browse the repository at this point in the history
In retrospect, while a nasty bug, it is not the library's responsibility
to detect and quietly fallback. The application would still be broken in
all other usages of the FJP commonPool. We should only sympathize with
affected users and inform them of a likely culprit. Ideally their system
heath tools would be able to detect probematic configurations like this.
  • Loading branch information
ben-manes committed Dec 2, 2021
1 parent 1405bd0 commit e32f711
Show file tree
Hide file tree
Showing 6 changed files with 4 additions and 49 deletions.
15 changes: 0 additions & 15 deletions .github/workflows/build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -63,18 +63,3 @@ jobs:
ORG_GRADLE_PROJECT_signingKey: ${{ secrets.OSSRH_GPG_SECRET_KEY }}
ORG_GRADLE_PROJECT_signingPassword: ${{ secrets.OSSRH_GPG_SECRET_KEY_PASSWORD }}
run: ./gradlew publishToSonatype
jdk8274349:
runs-on: ubuntu-latest
env:
JAVA_VERSION: 17
steps:
- uses: actions/checkout@v2.4.0
- name: Set up JDK 17.0.1
uses: actions/setup-java@v2
with:
distribution: 'zulu'
java-version: 17.0.1
- name: Run JDK-8274349 tests
uses: gradle/gradle-build-action@v2
with:
arguments: jdk8274349Test
Original file line number Diff line number Diff line change
Expand Up @@ -1455,7 +1455,7 @@ void afterWrite(Runnable task) {
scheduleDrainBuffers();
}

// The maintenance task may be scheduled but not running due. This might occur due to all of the
// The maintenance task may be scheduled but not running. This might occur due to all of the
// executor's threads being busy (perhaps writing into this cache), the write rate greatly
// exceeds the consuming rate, priority inversion, or if the executor silently discarded the
// maintenance task. In these scenarios then the writing threads cannot make progress and
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -300,14 +300,7 @@ public Caffeine<K, V> executor(Executor executor) {
}

Executor getExecutor() {
if ((executor != null)
&& !((executor instanceof ForkJoinPool) && (executor == ForkJoinPool.commonPool()))) {
return executor;
} else if ((ForkJoinPool.getCommonPoolParallelism() == 1) && (Runtime.version().feature() == 17)
&& (Runtime.version().interim() == 0) && (Runtime.version().update() <= 1)) {
return Runnable::run; // JDK-8274349
}
return ForkJoinPool.commonPool();
return (executor == null) ? ForkJoinPool.commonPool() : executor;
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,7 @@
import static com.google.common.util.concurrent.MoreExecutors.directExecutor;
import static org.mockito.Mockito.verify;

import java.lang.Runtime.Version;
import java.time.Duration;
import java.util.concurrent.ForkJoinPool;
import java.util.concurrent.TimeUnit;
import java.util.function.Supplier;

Expand All @@ -33,7 +31,6 @@
import org.testng.annotations.Test;

import com.github.benmanes.caffeine.cache.stats.StatsCounter;
import com.google.common.collect.Range;
import com.google.common.testing.FakeTicker;

/**
Expand Down Expand Up @@ -624,17 +621,6 @@ public void executor() {
builder.build();
}

@Test(groups = "jdk8274349")
public void executor_jdk8274349() throws ReflectiveOperationException {
// This test requires either JDK 17 or 17.0.1 using a single cpu (-XX:ActiveProcessorCount=1)
assertThat(Runtime.version()).isIn(Range.closedOpen(
Version.parse("17"), Version.parse("17.0.2")));
assertThat(Runtime.getRuntime().availableProcessors()).isEqualTo(1);

// JDK-8274349: ForkJoinPool.commonPool() does not work with 1 cpu
assertThat(Caffeine.newBuilder().getExecutor()).isNotSameInstanceAs(ForkJoinPool.commonPool());
}

/* --------------- ticker --------------- */

@Test(expectedExceptions = NullPointerException.class)
Expand Down
11 changes: 1 addition & 10 deletions caffeine/testing.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -74,12 +74,6 @@ tasks.register('junitTest', Test) {
systemProperty 'caffeine.osgi.jar', project(':caffeine').jar.archivePath.path
}

tasks.register('jdk8274349Test', Test) {
group = 'Cache tests'
description = 'JDK-8274349 tests'
useTestNG()
}

tasks.withType(Test).configureEach {
if (options instanceof TestNGOptions) {
if (name.startsWith('slow')) {
Expand All @@ -97,13 +91,10 @@ tasks.withType(Test).configureEach {
testLogging.events 'started'
maxHeapSize = '2g'
failFast = true
} else if (name.startsWith('jdk8274349')) {
options.includeGroups = ['jdk8274349']
jvmArgs '-XX:ActiveProcessorCount=1'
} else {
options {
threadCount = Math.max(6, Runtime.getRuntime().availableProcessors() - 1)
excludeGroups = ['slow', 'isolated', 'lincheck', 'jdk8274349']
excludeGroups = ['slow', 'isolated', 'lincheck']
parallel = 'methods'
}
jvmArgs '-XX:+UseG1GC', '-XX:+ParallelRefProcEnabled'
Expand Down
2 changes: 1 addition & 1 deletion gradle/wrapper/gradle-wrapper.properties
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
distributionUrl=https\://services.gradle.org/distributions/gradle-7.3-bin.zip
distributionUrl=https\://services.gradle.org/distributions/gradle-7.3.1-bin.zip
distributionBase=GRADLE_USER_HOME
distributionPath=wrapper/dists
zipStoreBase=GRADLE_USER_HOME
Expand Down

0 comments on commit e32f711

Please sign in to comment.