MATH-1693: Synchronize BaseRuleFactory.getRule

commons-math-legacy/src/main/java/org/apache/commons/math4/legacy/analysis/integration/gauss/BaseRuleFactory.java

@@ -52,7 +52,7 @@ public abstract class BaseRuleFactory<T extends Number> {
      * @throws DimensionMismatchException if the elements of the rule pair do not
      * have the same length.
      */
-    public Pair<double[], double[]> getRule(int numberOfPoints) {
+    public synchronized Pair<double[], double[]> getRule(int numberOfPoints) {
 
         if (numberOfPoints <= 0) {
             throw new NotStrictlyPositiveException(LocalizedFormats.NUMBER_OF_POINTS,

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<Double> 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<Future<?>> 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<Double> {
+        @Override
+        protected Pair<Double[], Double[]> 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<Pair<double[], double[]>> {