From 38f75fbfffabd5b55ed0b8953c26d8fcf380234e Mon Sep 17 00:00:00 2001 From: PJ Fanning Date: Mon, 7 Sep 2026 14:19:21 +0100 Subject: [PATCH] Release the Locale when a SAX load is abandoned SaxLoader.postLoad() nulls _locale and _context - "fix garbage collection of Locale -> Xobj -> STL" - but it only runs when the parse completes. Every failure path leaves both set: the four catch blocks abort the context and rethrow without clearing them, and an IOException out of _xr.parse() is not caught at all, so it does not even abort. The XMLReader holds the SaxLoader as its content, DTD, error, lexical and declaration handler. With a caller-supplied reader (XmlOptions.setLoadUseXMLReader) that reader outlives the parse, so a failed load pins the partly built document and its SchemaTypeLoader until the next parse on that reader. Abort and clear from a finally instead, which also covers the IOException path. The per-catch _context.abort() calls go away, and the RuntimeException catch existed only to abort before rethrowing. Co-Authored-By: Claude Opus 5 (1M context) --- .../apache/xmlbeans/impl/store/Locale.java | 30 ++++--- .../impl/store/SaxLoaderAbortTest.java | 87 +++++++++++++++++++ 2 files changed, 107 insertions(+), 10 deletions(-) create mode 100644 src/test/java/org/apache/xmlbeans/impl/store/SaxLoaderAbortTest.java diff --git a/src/main/java/org/apache/xmlbeans/impl/store/Locale.java b/src/main/java/org/apache/xmlbeans/impl/store/Locale.java index 38991a36e..5093ccd5e 100755 --- a/src/main/java/org/apache/xmlbeans/impl/store/Locale.java +++ b/src/main/java/org/apache/xmlbeans/impl/store/Locale.java @@ -2573,12 +2573,27 @@ void postLoad(Cur c) { _context = null; } + /** + * Drops a partly built document. The XMLReader holds this handler, and a + * caller-supplied one (XmlOptions.setLoadUseXMLReader) outlives the parse, so + * an abandoned load must let go of the Locale for the same reason postLoad does. + */ + private void abortLoad() { + if (_context != null) { + _context.abort(); + } + _locale = null; + _context = null; + } + public Cur load(Locale l, InputSource is, XmlOptions options) throws XmlException, IOException { is.setSystemId("file://"); initSaxHandler(l, options); + boolean loaded = false; + try { _xr.parse(is); @@ -2587,15 +2602,12 @@ public Cur load(Locale l, InputSource is, XmlOptions options) associateSourceName(c, options); postLoad(c); + loaded = true; return c; } catch (XmlRuntimeException e) { - _context.abort(); - throw new XmlException(e); } catch (SAXParseException e) { - _context.abort(); - XmlError err = XmlError.forLocation(e.getMessage(), options == null ? null : options.getDocumentSourceName(), @@ -2603,15 +2615,13 @@ public Cur load(Locale l, InputSource is, XmlOptions options) throw new XmlException(err.toString(), e, err); } catch (SAXException e) { - _context.abort(); - XmlError err = XmlError.forMessage(e.getMessage()); throw new XmlException(err.toString(), e, err); - } catch (RuntimeException e) { - _context.abort(); - - throw e; + } finally { + if (!loaded) { + abortLoad(); + } } } diff --git a/src/test/java/org/apache/xmlbeans/impl/store/SaxLoaderAbortTest.java b/src/test/java/org/apache/xmlbeans/impl/store/SaxLoaderAbortTest.java new file mode 100644 index 000000000..d10e978fd --- /dev/null +++ b/src/test/java/org/apache/xmlbeans/impl/store/SaxLoaderAbortTest.java @@ -0,0 +1,87 @@ +/* Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.xmlbeans.impl.store; + +import org.apache.xmlbeans.XmlException; +import org.apache.xmlbeans.XmlObject; +import org.apache.xmlbeans.XmlOptions; +import org.apache.xmlbeans.impl.common.SAXHelper; +import org.junit.jupiter.api.Test; +import org.xml.sax.ContentHandler; +import org.xml.sax.XMLReader; + +import java.lang.reflect.Field; + +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertThrows; + +/** + * A caller-supplied XMLReader outlives the parse and holds the SaxLoader as its + * content handler, so anything the loader still points at stays reachable. A load that + * completes clears that in postLoad(); one that fails has to do the same. + */ +public class SaxLoaderAbortTest { + + private static Object fieldValue(Object target, String name) throws Exception { + for (Class c = target.getClass(); c != null; c = c.getSuperclass()) { + try { + Field f = c.getDeclaredField(name); + f.setAccessible(true); + return f.get(target); + } catch (NoSuchFieldException ignored) { + // keep walking up + } + } + throw new NoSuchFieldException(name); + } + + private static void assertReaderReleasedItsDocument(String xml, boolean expectFailure) + throws Exception { + XMLReader xr = SAXHelper.newXMLReader(new XmlOptions()); + XmlOptions options = new XmlOptions().setLoadUseXMLReader(xr); + + if (expectFailure) { + assertThrows(XmlException.class, () -> XmlObject.Factory.parse(xml, options)); + } else { + assertNotNull(XmlObject.Factory.parse(xml, options)); + } + + ContentHandler handler = xr.getContentHandler(); + assertNotNull(handler, "the reader should still hold the loader"); + + assertNull(fieldValue(handler, "_locale"), + "the loader still points at the Locale, keeping the document and its type loader alive"); + assertNull(fieldValue(handler, "_context"), + "the loader still points at the load context"); + } + + @Test + void releasesTheDocumentAfterAFailedParse() throws Exception { + assertReaderReleasedItsDocument("", true); + } + + @Test + void releasesTheDocumentAfterAParseErrorPartWayIn() throws Exception { + assertReaderReleasedItsDocument("", true); + } + + @Test + void releasesTheDocumentAfterASuccessfulParse() throws Exception { + assertReaderReleasedItsDocument("", false); + } +}