diff --git a/commons-math-legacy/src/main/java/org/apache/commons/math4/legacy/stat/descriptive/SynchronizedDescriptiveStatistics.java b/commons-math-legacy/src/main/java/org/apache/commons/math4/legacy/stat/descriptive/SynchronizedDescriptiveStatistics.java index 0fc06af95..486f91151 100644 --- a/commons-math-legacy/src/main/java/org/apache/commons/math4/legacy/stat/descriptive/SynchronizedDescriptiveStatistics.java +++ b/commons-math-legacy/src/main/java/org/apache/commons/math4/legacy/stat/descriptive/SynchronizedDescriptiveStatistics.java @@ -85,6 +85,22 @@ public synchronized void clear() { super.clear(); } + /** + * {@inheritDoc} + */ + @Override + public synchronized void removeMostRecentValue() { + super.removeMostRecentValue(); + } + + /** + * {@inheritDoc} + */ + @Override + public synchronized double replaceMostRecentValue(double v) { + return super.replaceMostRecentValue(v); + } + /** * {@inheritDoc} */ diff --git a/commons-math-legacy/src/test/java/org/apache/commons/math4/legacy/stat/descriptive/SynchronizedDescriptiveStatisticsTest.java b/commons-math-legacy/src/test/java/org/apache/commons/math4/legacy/stat/descriptive/SynchronizedDescriptiveStatisticsTest.java index 512063ab3..5f77f6188 100644 --- a/commons-math-legacy/src/test/java/org/apache/commons/math4/legacy/stat/descriptive/SynchronizedDescriptiveStatisticsTest.java +++ b/commons-math-legacy/src/test/java/org/apache/commons/math4/legacy/stat/descriptive/SynchronizedDescriptiveStatisticsTest.java @@ -16,6 +16,12 @@ */ package org.apache.commons.math4.legacy.stat.descriptive; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.TimeUnit; + +import org.junit.Assert; +import org.junit.Test; + /** * Test cases for the {@link SynchronizedDescriptiveStatisticsTest} class. * 2007) $ @@ -26,4 +32,55 @@ public final class SynchronizedDescriptiveStatisticsTest extends DescriptiveStat protected DescriptiveStatistics createDescriptiveStatistics() { return new SynchronizedDescriptiveStatistics(); } + + /** + * A state-touching method of {@link SynchronizedDescriptiveStatistics} must + * acquire the instance monitor, so a call from another thread has to block + * while the monitor is held. The methods exercised below used not to be + * overridden, so they ran without the lock. + * + * @param call the method under test. + * @throws InterruptedException if the test is interrupted while waiting. + */ + private void checkLocksOnInstance(final MethodCall call) throws InterruptedException { + final SynchronizedDescriptiveStatistics stats = new SynchronizedDescriptiveStatistics(); + stats.addValue(1); + stats.addValue(2); + stats.addValue(3); + + final CountDownLatch started = new CountDownLatch(1); + final CountDownLatch finished = new CountDownLatch(1); + final Thread worker; + synchronized (stats) { + worker = new Thread(() -> { + started.countDown(); + call.apply(stats); + finished.countDown(); + }); + worker.start(); + // Make sure the worker is running before we check that it is blocked. + started.await(); + Assert.assertFalse("method ran without holding the instance lock", + finished.await(500, TimeUnit.MILLISECONDS)); + } + worker.join(); + } + + @Test + public void testRemoveMostRecentValueIsSynchronized() throws InterruptedException { + checkLocksOnInstance(DescriptiveStatistics::removeMostRecentValue); + } + + @Test + public void testReplaceMostRecentValueIsSynchronized() throws InterruptedException { + checkLocksOnInstance(s -> s.replaceMostRecentValue(4)); + } + + /** A call taking a {@link DescriptiveStatistics} instance. */ + private interface MethodCall { + /** + * @param stats the instance to operate on. + */ + void apply(DescriptiveStatistics stats); + } }