On Mon, Jul 20, 2026 at 5:52 PM Mark Thomas <[email protected]> wrote: > > On 20/07/2026 15:42, Rémy Maucherat wrote: > > 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 wh > > 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. > > That looks like an hallucination. HashSet.spliterator() does not have an > iterator. Nor does it have a remove() method. > > I don't see a way to mutate the underlying Set via the object returned > from spliterator(). > > I'm going to remove that code as unnecessary.
+1 Rémy --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
