From 9b68d72994ad7f6de146313a6b46c77acf25150b Mon Sep 17 00:00:00 2001 From: Mahammad Insaf Date: Mon, 26 Oct 2026 21:59:13 +0530 Subject: [PATCH 1/3] fix racy cache lookups in BaseRuleFactory and PolynomialsUtils BaseRuleFactory.getRule read and updated the pointsAndWeightsDouble TreeMap without holding the lock used by getRuleInternal. Factory instances are shared (IterativeLegendreGaussIntegrator keeps a static GaussIntegratorFactory), so concurrent requests for different rules could corrupt the map and leave callers spinning in TreeMap.getEntry. PolynomialsUtils.createJacobiPolynomial published the coefficient list for a new (v, w) key before seeding it and then read it outside the lock guarding the seeding, so a racing caller could extend a partial list and return a wrong polynomial. buildPolynomial also read the cached coefficients after releasing the list lock. --- .../integration/gauss/BaseRuleFactory.java | 2 +- .../polynomials/PolynomialsUtils.java | 66 ++++++++++--------- .../polynomials/PolynomialsUtilsTest.java | 39 +++++++++++ 3 files changed, 74 insertions(+), 33 deletions(-) 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/main/java/org/apache/commons/math4/legacy/analysis/polynomials/PolynomialsUtils.java b/commons-math-legacy/src/main/java/org/apache/commons/math4/legacy/analysis/polynomials/PolynomialsUtils.java index 01648b615b..8fda4231bd 100644 --- a/commons-math-legacy/src/main/java/org/apache/commons/math4/legacy/analysis/polynomials/PolynomialsUtils.java +++ b/commons-math-legacy/src/main/java/org/apache/commons/math4/legacy/analysis/polynomials/PolynomialsUtils.java @@ -228,24 +228,26 @@ public static PolynomialFunction createJacobiPolynomial(final int degree, final // select the appropriate list final JacobiKey key = new JacobiKey(v, w); - if (!JACOBI_COEFFICIENTS.containsKey(key)) { - synchronized(JACOBI_COEFFICIENTS) { - if (!JACOBI_COEFFICIENTS.containsKey(key)) { - // allocate a new list for v, w - final List list = new ArrayList<>(); - JACOBI_COEFFICIENTS.put(key, list); - - // Pv,w,0(x) = 1; - list.add(BigFraction.ONE); - - // P1(x) = (v - w) / 2 + (2 + v + w) * X / 2 - list.add(BigFraction.of(v - w, 2)); - list.add(BigFraction.of(2 + v + w, 2)); - } + final List coefficients; + synchronized(JACOBI_COEFFICIENTS) { + List list = JACOBI_COEFFICIENTS.get(key); + if (list == null) { + // allocate a new list for v, w + list = new ArrayList<>(); + + // Pv,w,0(x) = 1; + list.add(BigFraction.ONE); + + // P1(x) = (v - w) / 2 + (2 + v + w) * X / 2 + list.add(BigFraction.of(v - w, 2)); + list.add(BigFraction.of(2 + v + w, 2)); + + JACOBI_COEFFICIENTS.put(key, list); } + coefficients = list; } - return buildPolynomial(degree, JACOBI_COEFFICIENTS.get(key), + return buildPolynomial(degree, coefficients, new RecurrenceCoefficientsGenerator() { /** {@inheritDoc} */ @Override @@ -373,25 +375,25 @@ private static PolynomialFunction buildPolynomial(final int degree, if (degree > maxDegree) { computeUpToDegree(degree, maxDegree, generator, coefficients); } - } - // coefficient for polynomial 0 is l [0] - // coefficients for polynomial 1 are l [1] ... l [2] (degrees 0 ... 1) - // coefficients for polynomial 2 are l [3] ... l [5] (degrees 0 ... 2) - // coefficients for polynomial 3 are l [6] ... l [9] (degrees 0 ... 3) - // coefficients for polynomial 4 are l[10] ... l[14] (degrees 0 ... 4) - // coefficients for polynomial 5 are l[15] ... l[20] (degrees 0 ... 5) - // coefficients for polynomial 6 are l[21] ... l[27] (degrees 0 ... 6) - // ... - final int start = degree * (degree + 1) / 2; - - final double[] a = new double[degree + 1]; - for (int i = 0; i <= degree; ++i) { - a[i] = coefficients.get(start + i).doubleValue(); - } + // coefficient for polynomial 0 is l [0] + // coefficients for polynomial 1 are l [1] ... l [2] (degrees 0 ... 1) + // coefficients for polynomial 2 are l [3] ... l [5] (degrees 0 ... 2) + // coefficients for polynomial 3 are l [6] ... l [9] (degrees 0 ... 3) + // coefficients for polynomial 4 are l[10] ... l[14] (degrees 0 ... 4) + // coefficients for polynomial 5 are l[15] ... l[20] (degrees 0 ... 5) + // coefficients for polynomial 6 are l[21] ... l[27] (degrees 0 ... 6) + // ... + final int start = degree * (degree + 1) / 2; + + final double[] a = new double[degree + 1]; + for (int i = 0; i <= degree; ++i) { + a[i] = coefficients.get(start + i).doubleValue(); + } - // build the polynomial - return new PolynomialFunction(a); + // build the polynomial + return new PolynomialFunction(a); + } } /** Compute polynomial coefficients up to a given degree. diff --git a/commons-math-legacy/src/test/java/org/apache/commons/math4/legacy/analysis/polynomials/PolynomialsUtilsTest.java b/commons-math-legacy/src/test/java/org/apache/commons/math4/legacy/analysis/polynomials/PolynomialsUtilsTest.java index d6f4014f72..1f6732c6fb 100644 --- a/commons-math-legacy/src/test/java/org/apache/commons/math4/legacy/analysis/polynomials/PolynomialsUtilsTest.java +++ b/commons-math-legacy/src/test/java/org/apache/commons/math4/legacy/analysis/polynomials/PolynomialsUtilsTest.java @@ -16,6 +16,13 @@ */ package org.apache.commons.math4.legacy.analysis.polynomials; +import java.util.ArrayList; +import java.util.List; +import java.util.concurrent.CyclicBarrier; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.Future; + import org.apache.commons.math4.legacy.analysis.UnivariateFunction; import org.apache.commons.math4.legacy.analysis.integration.IterativeLegendreGaussIntegrator; import org.apache.commons.numbers.combinatorics.BinomialCoefficient; @@ -291,6 +298,38 @@ public void testJacobiEvaluationAt1() { } } + @Test + public void testJacobiConcurrentCreation() throws Exception { + final int numThreads = 8; + final ExecutorService exec = Executors.newFixedThreadPool(numThreads); + try { + // Exponents not used elsewhere, so that every (v, w) list is + // created while the threads are racing. + for (int v = 20; v < 40; ++v) { + for (int w = 20; w < 40; ++w) { + final int fv = v; + final int fw = w; + final CyclicBarrier barrier = new CyclicBarrier(numThreads); + final List> results = new ArrayList<>(); + for (int t = 0; t < numThreads; ++t) { + final int degree = 1 + t % 4; + results.add(exec.submit(() -> { + barrier.await(); + return PolynomialsUtils.createJacobiPolynomial(degree, fv, fw).value(1.0); + })); + } + for (int t = 0; t < numThreads; ++t) { + final int degree = 1 + t % 4; + final double binomial = BinomialCoefficient.value(v + degree, degree); + Assert.assertTrue(Precision.equals(binomial, results.get(t).get(), 1)); + } + } + } + } finally { + exec.shutdownNow(); + } + } + @Test public void testJacobiOrthogonality() { for (int v = 0; v < 5; ++v) { From 665cafb7f6dbff6dcc9a5f8f70146afcc35028f3 Mon Sep 17 00:00:00 2001 From: Mahammad Insaf Date: Wed, 28 Oct 2026 09:54:53 +0530 Subject: [PATCH 2/3] Use ConcurrentHashMap for Jacobi cache and build polynomial outside lock --- .../polynomials/PolynomialsUtils.java | 38 +++++++++---------- 1 file changed, 17 insertions(+), 21 deletions(-) diff --git a/commons-math-legacy/src/main/java/org/apache/commons/math4/legacy/analysis/polynomials/PolynomialsUtils.java b/commons-math-legacy/src/main/java/org/apache/commons/math4/legacy/analysis/polynomials/PolynomialsUtils.java index 8fda4231bd..552dbb94f7 100644 --- a/commons-math-legacy/src/main/java/org/apache/commons/math4/legacy/analysis/polynomials/PolynomialsUtils.java +++ b/commons-math-legacy/src/main/java/org/apache/commons/math4/legacy/analysis/polynomials/PolynomialsUtils.java @@ -17,9 +17,9 @@ package org.apache.commons.math4.legacy.analysis.polynomials; import java.util.ArrayList; -import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.concurrent.ConcurrentHashMap; import org.apache.commons.numbers.fraction.BigFraction; import org.apache.commons.numbers.combinatorics.BinomialCoefficient; @@ -82,7 +82,7 @@ public final class PolynomialsUtils { LEGENDRE_COEFFICIENTS.add(BigFraction.ONE); // initialize map for Jacobi polynomials - JACOBI_COEFFICIENTS = new HashMap<>(); + JACOBI_COEFFICIENTS = new ConcurrentHashMap<>(); } /** @@ -228,24 +228,19 @@ public static PolynomialFunction createJacobiPolynomial(final int degree, final // select the appropriate list final JacobiKey key = new JacobiKey(v, w); - final List coefficients; - synchronized(JACOBI_COEFFICIENTS) { - List list = JACOBI_COEFFICIENTS.get(key); - if (list == null) { - // allocate a new list for v, w - list = new ArrayList<>(); + final List coefficients = JACOBI_COEFFICIENTS.computeIfAbsent(key, k -> { + // allocate a new list for v, w + final List list = new ArrayList<>(); - // Pv,w,0(x) = 1; - list.add(BigFraction.ONE); + // Pv,w,0(x) = 1; + list.add(BigFraction.ONE); - // P1(x) = (v - w) / 2 + (2 + v + w) * X / 2 - list.add(BigFraction.of(v - w, 2)); - list.add(BigFraction.of(2 + v + w, 2)); + // P1(x) = (v - w) / 2 + (2 + v + w) * X / 2 + list.add(BigFraction.of(v - w, 2)); + list.add(BigFraction.of(2 + v + w, 2)); - JACOBI_COEFFICIENTS.put(key, list); - } - coefficients = list; - } + return list; + }); return buildPolynomial(degree, coefficients, new RecurrenceCoefficientsGenerator() { @@ -370,6 +365,7 @@ private static PolynomialFunction buildPolynomial(final int degree, // Synchronizing on a method parameter is not safe; however, in this // case, the lock object is an immutable field that belongs to this // class. + final double[] a; synchronized (coefficients) { final int maxDegree = (int) JdkMath.floor(JdkMath.sqrt(2.0 * coefficients.size())) - 1; if (degree > maxDegree) { @@ -386,14 +382,14 @@ private static PolynomialFunction buildPolynomial(final int degree, // ... final int start = degree * (degree + 1) / 2; - final double[] a = new double[degree + 1]; + a = new double[degree + 1]; for (int i = 0; i <= degree; ++i) { a[i] = coefficients.get(start + i).doubleValue(); } - - // build the polynomial - return new PolynomialFunction(a); } + + // build the polynomial + return new PolynomialFunction(a); } /** Compute polynomial coefficients up to a given degree. From 618cf4d30e93cc2db5403c2e630b88a12cd9f092 Mon Sep 17 00:00:00 2001 From: Mahammad Insaf Date: Thu, 29 Oct 2026 09:55:08 +0530 Subject: [PATCH 3/3] Drop BaseRuleFactory change, tracked separately as MATH-1693 --- .../legacy/analysis/integration/gauss/BaseRuleFactory.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 c82fe74f2c..13d04d6ee7 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 synchronized Pair getRule(int numberOfPoints) { + public Pair getRule(int numberOfPoints) { if (numberOfPoints <= 0) { throw new NotStrictlyPositiveException(LocalizedFormats.NUMBER_OF_POINTS,