diff --git a/commons-math-legacy/src/main/java/org/apache/commons/math4/legacy/analysis/integration/gauss/BaseRuleFactory.java b/commons-math-legacy/src/main/java/org/apache/commons/math4/legacy/analysis/integration/gauss/BaseRuleFactory.java index 13d04d6ee7..c82fe74f2c 100644 --- a/commons-math-legacy/src/main/java/org/apache/commons/math4/legacy/analysis/integration/gauss/BaseRuleFactory.java +++ b/commons-math-legacy/src/main/java/org/apache/commons/math4/legacy/analysis/integration/gauss/BaseRuleFactory.java @@ -52,7 +52,7 @@ public abstract class BaseRuleFactory { * @throws DimensionMismatchException if the elements of the rule pair do not * have the same length. */ - public Pair getRule(int numberOfPoints) { + public synchronized Pair getRule(int numberOfPoints) { if (numberOfPoints <= 0) { throw new NotStrictlyPositiveException(LocalizedFormats.NUMBER_OF_POINTS, diff --git a/commons-math-legacy/src/test/java/org/apache/commons/math4/legacy/analysis/integration/gauss/BaseRuleFactoryTest.java b/commons-math-legacy/src/test/java/org/apache/commons/math4/legacy/analysis/integration/gauss/BaseRuleFactoryTest.java index 89ede23ea4..8b3ab3e3a1 100644 --- a/commons-math-legacy/src/test/java/org/apache/commons/math4/legacy/analysis/integration/gauss/BaseRuleFactoryTest.java +++ b/commons-math-legacy/src/test/java/org/apache/commons/math4/legacy/analysis/integration/gauss/BaseRuleFactoryTest.java @@ -16,8 +16,12 @@ */ package org.apache.commons.math4.legacy.analysis.integration.gauss; +import java.time.Duration; import java.util.List; import java.util.ArrayList; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; import java.util.concurrent.ThreadPoolExecutor; import java.util.concurrent.ArrayBlockingQueue; import java.util.concurrent.TimeUnit; @@ -29,6 +33,7 @@ import org.apache.commons.math4.legacy.core.Pair; import org.junit.Test; import org.junit.Assert; +import org.junit.jupiter.api.Assertions; /** * Test for {@link BaseRuleFactory}. @@ -65,6 +70,73 @@ public void testConcurrentCreation() throws InterruptedException, final int n = RuleBuilder.getNumberOfCalls(); Assert.assertEquals("Rule computation was called " + n + " times", 1, n); } + + /** + * Tests that concurrent requests for different rules on a shared factory + * do not corrupt the cache of double-precision rules (MATH-1693). + * Without synchronization of "getRule", the cache can be left in a state + * where a lookup never terminates. + */ + @Test + public void testConcurrentCreationOfDifferentRules() { + final int numThreads = 8; + final int maxPoints = 1000; + final int rounds = 20; + + Assertions.assertTimeoutPreemptively(Duration.ofSeconds(30), () -> { + final ExecutorService exec = Executors.newFixedThreadPool(numThreads, r -> { + final Thread t = new Thread(r); + t.setDaemon(true); + return t; + }); + try { + for (int r = 0; r < rounds; r++) { + final BaseRuleFactory factory = new CheapRuleFactory(); + // Compute the rules upfront so that the threads mostly race + // on the double-precision cache. + for (int n = 1; n <= maxPoints; n++) { + factory.getRuleInternal(n); + } + final CountDownLatch start = new CountDownLatch(1); + final List> results = new ArrayList<>(); + for (int t = 0; t < numThreads; t++) { + final int offset = t; + results.add(exec.submit(() -> { + start.await(); + for (int i = 0; i < maxPoints; i++) { + final int n = 1 + (i * numThreads + offset) % maxPoints; + Assert.assertEquals(n, factory.getRule(n).getFirst().length); + } + return null; + })); + } + start.countDown(); + for (Future f : results) { + f.get(); + } + for (int n = 1; n <= maxPoints; n++) { + Assert.assertEquals(n, factory.getRule(n).getFirst().length); + } + } + } finally { + exec.shutdownNow(); + } + }); + } + + /** Factory with a fast rule computation. */ + private static final class CheapRuleFactory extends BaseRuleFactory { + @Override + protected Pair computeRule(int order) { + final Double[] p = new Double[order]; + final Double[] w = new Double[order]; + for (int i = 0; i < order; i++) { + p[i] = Double.valueOf(i); + w[i] = Double.valueOf(i); + } + return new Pair<>(p, w); + } + } } class RuleBuilder implements Callable> {