synchronize missing mutators in SynchronizedDescriptiveStatistics

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}
      */

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);
+    }
 }