On Mon, Jul 20, 2026 at 12:01 PM Mark Thomas <[email protected]> wrote: > > On 15/07/2026 16:06, [email protected] wrote: > > This is an automated email from the ASF dual-hosted git repository. > > > > rmaucher pushed a commit to branch main > > in repository https://gitbox.apache.org/repos/asf/tomcat.git > > > > > > The following commit(s) were added to refs/heads/main by this push: > > new 60008aaaf7 Code review fixes > > 60008aaaf7 is described below > > > > commit 60008aaaf73a5e864f382f4667d576ea2d5b2f2a > > Author: remm <[email protected]> > > AuthorDate: Wed Jul 15 17:06:29 2026 +0200 > > > > Code review fixes > > <snip/> > > > diff --git a/java/org/apache/catalina/util/ResourceSet.java > > b/java/org/apache/catalina/util/ResourceSet.java > > index f7a82f7451..8fcc5cdc81 100644 > > --- a/java/org/apache/catalina/util/ResourceSet.java > > +++ b/java/org/apache/catalina/util/ResourceSet.java > > <snip/> > > > @@ -221,6 +222,23 @@ public final class ResourceSet<T> extends HashSet<T> { > > } > > > > > > + @Override > > + public java.util.Spliterator<T> spliterator() { > > + if (locked) { > > + java.util.Spliterator<T> base = super.spliterator(); > > + return new java.util.Spliterator<T>() { > > + @Override public boolean > > tryAdvance(java.util.function.Consumer<? super T> action) { return > > base.tryAdvance(action); } > > + @Override public java.util.Spliterator<T> trySplit() { > > return null; } > > + @Override public long estimateSize() { return > > base.estimateSize(); } > > + @Override public int characteristics() { return > > base.characteristics() & ~java.util.Spliterator.SORTED; } > > + @Override public void > > forEachRemaining(java.util.function.Consumer<? super T> action) { > > base.forEachRemaining(action); } > > + @Override public Comparator<? super T> getComparator() { > > return base.getComparator(); } > > + }; > > + } > > + return super.spliterator(); > > + } > > What was the intention here? The above code is a NO-OP. ResourceSet > extends HashSet which never returns the SORTED characteristic. Also, it > is not clear why this is conditional on the status of locked. > > I did wonder if the intention was to add IMMTABLE when the Set was > locked but the logic for that is sufficiently different that I am really > not sure.
Code review had a defect like this: "ResourceSet overrides iterator() to return a LockedIterator when locked (line 230-235), which throws on remove(). However, spliterator() is not overridden. When the set is locked, calling spliterator() returns the inherited HashSet.spliterator(), whose iterator allows remove() operations, bypassing the lock." I don't have experience using spliterator. iterator() returns a LockedIterator when locked, and a regular iterator otherwise. Rémy --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
