From da3fe232230be0cb087101e3e3ec3f5240ffdcd5 Mon Sep 17 00:00:00 2001 From: Naveed Khan Date: Sat, 1 Aug 2026 14:46:16 +0530 Subject: [PATCH 1/2] synchronize forEach in SynchronizedCollection --- .../collection/SynchronizedCollection.java | 11 +++++ .../SynchronizedCollectionTest.java | 48 +++++++++++++++++++ 2 files changed, 59 insertions(+) diff --git a/src/main/java/org/apache/commons/collections4/collection/SynchronizedCollection.java b/src/main/java/org/apache/commons/collections4/collection/SynchronizedCollection.java index cefeddbbd3..0f01117f7f 100644 --- a/src/main/java/org/apache/commons/collections4/collection/SynchronizedCollection.java +++ b/src/main/java/org/apache/commons/collections4/collection/SynchronizedCollection.java @@ -20,6 +20,7 @@ import java.util.Collection; import java.util.Iterator; import java.util.Objects; +import java.util.function.Consumer; import java.util.function.Predicate; /** @@ -142,6 +143,16 @@ public boolean equals(final Object object) { } } + /** + * @since 4.6.0 + */ + @Override + public void forEach(final Consumer action) { + synchronized (lock) { + decorated().forEach(action); + } + } + @Override public int hashCode() { synchronized (lock) { diff --git a/src/test/java/org/apache/commons/collections4/collection/SynchronizedCollectionTest.java b/src/test/java/org/apache/commons/collections4/collection/SynchronizedCollectionTest.java index bfd265d256..6e171f85e0 100644 --- a/src/test/java/org/apache/commons/collections4/collection/SynchronizedCollectionTest.java +++ b/src/test/java/org/apache/commons/collections4/collection/SynchronizedCollectionTest.java @@ -16,9 +16,29 @@ */ package org.apache.commons.collections4.collection; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.junit.jupiter.params.provider.Arguments.arguments; + import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; +import java.util.LinkedList; +import java.util.List; +import java.util.stream.Stream; + +import org.apache.commons.collections4.bag.HashBag; +import org.apache.commons.collections4.bag.SynchronizedBag; +import org.apache.commons.collections4.bag.SynchronizedSortedBag; +import org.apache.commons.collections4.bag.TreeBag; +import org.apache.commons.collections4.multiset.HashMultiSet; +import org.apache.commons.collections4.multiset.SynchronizedMultiSet; +import org.apache.commons.collections4.multiset.SynchronizedSortedMultiSet; +import org.apache.commons.collections4.multiset.TreeMultiSet; +import org.apache.commons.collections4.queue.SynchronizedQueue; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; /** * Extension of {@link AbstractCollectionTest} for exercising the @@ -26,6 +46,22 @@ */ public class SynchronizedCollectionTest extends AbstractCollectionTest { + /** The elements every decorator under test is populated with. */ + private static final List ELEMENTS = Arrays.asList("a", "b"); + + /** + * Every decorator that inherits {@link SynchronizedCollection#forEach(java.util.function.Consumer)}. + */ + static Stream getSynchronizedDecorators() { + return Stream.of( + arguments("SynchronizedCollection", SynchronizedCollection.synchronizedCollection(new ArrayList<>(ELEMENTS))), + arguments("SynchronizedBag", SynchronizedBag.synchronizedBag(new HashBag<>(ELEMENTS))), + arguments("SynchronizedSortedBag", SynchronizedSortedBag.synchronizedSortedBag(new TreeBag<>(ELEMENTS))), + arguments("SynchronizedMultiSet", SynchronizedMultiSet.synchronizedMultiSet(new HashMultiSet<>(ELEMENTS))), + arguments("SynchronizedSortedMultiSet", SynchronizedSortedMultiSet.synchronizedSortedMultiSet(new TreeMultiSet<>(ELEMENTS))), + arguments("SynchronizedQueue", SynchronizedQueue.synchronizedQueue(new LinkedList<>(ELEMENTS)))); + } + @Override public String getCompatibilityVersion() { return "4"; @@ -46,6 +82,18 @@ public Collection makeObject() { return SynchronizedCollection.synchronizedCollection(new ArrayList<>()); } + @ParameterizedTest(name = "{0}") + @MethodSource("getSynchronizedDecorators") + void testForEachHoldsLock(final String description, final Collection decorator) { + final List visited = new ArrayList<>(); + decorator.forEach(element -> { + assertTrue(Thread.holdsLock(decorator), () -> description + " ran forEach without holding its lock"); + visited.add(element); + }); + assertEquals(ELEMENTS.size(), visited.size()); + assertTrue(visited.containsAll(ELEMENTS)); + } + // void testCreate() throws Exception { // resetEmpty(); // writeExternalFormToDisk((java.io.Serializable) getCollection(), "src/test/resources/data/test/SynchronizedCollection.emptyCollection.version4.obj"); From 75681c50531fa10dd19574ddec055286ea93a87c Mon Sep 17 00:00:00 2001 From: Gary Gregory Date: Sat, 1 Aug 2026 07:43:08 -0400 Subject: [PATCH 2/2] Clarify comment on elements used in tests --- .../collections4/collection/SynchronizedCollectionTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/java/org/apache/commons/collections4/collection/SynchronizedCollectionTest.java b/src/test/java/org/apache/commons/collections4/collection/SynchronizedCollectionTest.java index 6e171f85e0..2292f7245f 100644 --- a/src/test/java/org/apache/commons/collections4/collection/SynchronizedCollectionTest.java +++ b/src/test/java/org/apache/commons/collections4/collection/SynchronizedCollectionTest.java @@ -46,7 +46,7 @@ */ public class SynchronizedCollectionTest extends AbstractCollectionTest { - /** The elements every decorator under test is populated with. */ + /** The elements used to populate each decorator under test. */ private static final List ELEMENTS = Arrays.asList("a", "b"); /**