Cascade cache resets in StructuralEvaluator

Fixes #2277

Author: jhy

CHANGES.md

@@ -70,6 +70,8 @@
   positive step, and so would not match as expected. [1147](https://github.com/jhy/jsoup/issues/1147)
 * Calling `doc.charset(charset)` on an empty XML document would throw an
   `IndexOutOfBoundsException`. [2266](https://github.com/jhy/jsoup/issues/2266)
+* Fixed a memory leak when reusing a nested `StructuralEvaluator` (e.g., a selector ancestor chain like `A B C`) by
+  ensuring cache reset calls cascade to inner members. [2277](https://github.com/jhy/jsoup/issues/2277)
 
 ## 1.18.3 (2024-Dec-02)
 

src/main/java/org/jsoup/select/StructuralEvaluator.java

@@ -33,6 +33,7 @@ boolean memoMatches(final Element root, final Element element) {
 
     @Override protected void reset() {
         threadMemo.get().clear();
+        evaluator.reset();
         super.reset();
     }
 
@@ -221,6 +222,14 @@ public boolean matches(Element root, Element element) {
             return cost;
         }
 
+        @Override
+        protected void reset() {
+            for (Evaluator evaluator : evaluators) {
+                evaluator.reset();
+            }
+            super.reset();
+        }
+
         @Override
         public String toString() {
             return StringUtil.join(evaluators, " > ");

src/test/java/org/jsoup/select/SelectorIT.java

@@ -2,11 +2,14 @@
 
 import org.jsoup.Jsoup;
 import org.jsoup.nodes.Document;
+import org.jsoup.parser.Parser;
+import org.jsoup.parser.StreamParser;
 import org.junit.jupiter.api.Test;
 
 import java.util.concurrent.atomic.AtomicInteger;
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.fail;
 
 public class SelectorIT {
 
@@ -55,4 +58,39 @@ public void uncaughtException(Thread t, Throwable e) {
         }
     }
 
+    @Test public void streamParserSelect() throws Exception {
+        // https://github.com/jhy/jsoup/issues/2277
+        // The memo in the StructuralEvaluator was not getting reset correctly, and so would run out of memory
+        // Test tracks memory consumption. Will be interesting to see how it behaves on the CI workers.
+
+        String xml = "<A><B><C>1";
+        Evaluator query = QueryParser.parse("A B C");
+        Runtime runtime = Runtime.getRuntime();
+
+        System.gc();
+        Thread.sleep(100);
+        long initialUsed = runtime.totalMemory() - runtime.freeMemory();
+
+        for (int i = 0; i < 50_000; i++) { // Before fix, would exceed 10MB in ~ 9000 iters
+            try (StreamParser parser = new StreamParser(Parser.xmlParser())) {
+                parser.parse(xml, "");
+                parser.selectFirst(query);
+                parser.stop();
+            }
+
+            if (i % 1000 == 0) {
+                System.gc();
+                Thread.sleep(100);
+                long currentUsed = runtime.totalMemory() - runtime.freeMemory();
+                long delta = currentUsed - initialUsed;
+
+                // Fail if we grow + 10MB
+                if (delta > 10_000_000) {
+                    fail(String.format("Memo leak detected. Memory increased by %,d bytes after %,d iterations",
+                        delta, i));
+                }
+            }
+        }
+    }
+
 }

src/test/java/org/jsoup/select/SelectorTest.java

@@ -1381,4 +1381,73 @@ public void emptyPseudo() {
         Elements neg2 = doc.select("p:nth-child(-1n+2)");
         assertSelectedOwnText(neg2, "1", "2");
     }
+
+    // Tests that nested structural and combining evaluators get reset
+    private static class ResetTracker extends Evaluator {
+        boolean resetCalled = false;
+        @Override
+        public boolean matches(Element root, Element element) {
+            return true;
+        }
+
+        @Override
+        protected void reset() {
+            resetCalled = true;
+            super.reset();
+        }
+    }
+
+    @Test void notResetCascades() {
+        ResetTracker track = new ResetTracker();
+        StructuralEvaluator.Not structEval = new StructuralEvaluator.Not(track);
+
+        Document doc = Jsoup.parse("<div><p>Test</p></div>");
+        Element p = doc.expectFirst("p");
+        structEval.matches(doc, p);
+
+        assertFalse(structEval.threadMemo.get().isEmpty());
+        assertFalse(track.resetCalled);
+
+        structEval.reset();
+        assertTrue(structEval.threadMemo.get().isEmpty());
+        assertTrue(track.resetCalled);
+    }
+
+    @Test void testImmediateParentRunCascades() {
+        ResetTracker child = new ResetTracker();
+        ResetTracker parent = new ResetTracker();
+
+        StructuralEvaluator.ImmediateParentRun run = new StructuralEvaluator.ImmediateParentRun(child);
+        run.add(parent);
+
+        Document doc = Jsoup.parse("<div><p><span>Test</span></p></div>");
+        Element span = doc.expectFirst("span");
+        assertTrue(run.matches(doc, span));
+
+        run.reset();
+        assertTrue(child.resetCalled);
+        assertTrue(parent.resetCalled);
+    }
+
+    @Test
+    public void testAncestorChain() {
+        ResetTracker grandParent = new ResetTracker();
+        ResetTracker parent = new ResetTracker();
+        ResetTracker child = new ResetTracker();
+
+        StructuralEvaluator.Ancestor b_needs_a = new StructuralEvaluator.Ancestor(grandParent);
+        StructuralEvaluator.Ancestor c_needs_b = new StructuralEvaluator.Ancestor(parent);
+        CombiningEvaluator.And chain = new CombiningEvaluator.And(child, c_needs_b, b_needs_a);
+
+        Document doc = Jsoup.parse("<div class='A'><p class='B'><span class='C'>Test</span></p></div>");
+        Element span = doc.expectFirst("span");
+        assertTrue(chain.matches(doc, span), "Should match span in correct ancestor chain");
+
+        chain.reset();
+        assertTrue(grandParent.resetCalled);
+        assertTrue(parent.resetCalled);
+        assertTrue(child.resetCalled);
+        assertTrue(b_needs_a.threadMemo.get().isEmpty());
+        assertTrue(c_needs_b.threadMemo.get().isEmpty());
+    }
 }