From 56525e2ac05850ba317ef0c0fef36aa6c7f9c246 Mon Sep 17 00:00:00 2001 From: Red Davies Date: Sun, 11 Jan 2026 01:13:19 -0500 Subject: [PATCH] Fixes #2 by ensuring that Xml2Doc is last to die. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In our previous iteration of this API we had Pony Class instances freeing C structs which were still referenced by Xml2Doc. The documentation states that for our current parsing use-cases it is unsafe to automatically _final() free any of the XmlNodes as they're a part of our XmlDoc tree. As such, we have threaded a tag reference through every dependent Pony instance so that it will be "impossible"™ for Xml2Doc._final() to execute the frees until all other dependent references are no longer reachable. --- .release-notes/2.md | 7 +++++++ libxml2/xml2doc.pony | 12 ++++-------- libxml2/xml2node.pony | 10 ++++++---- libxml2/xml2xpathobject.pony | 4 ++-- 4 files changed, 19 insertions(+), 14 deletions(-) create mode 100644 .release-notes/2.md diff --git a/.release-notes/2.md b/.release-notes/2.md new file mode 100644 index 0000000..b9048b3 --- /dev/null +++ b/.release-notes/2.md @@ -0,0 +1,7 @@ +## Fixes #1 by ensuring that Xml2Doc is last to die + +In our previous iteration of this API we had Pony Class instances freeing C structs which were still referenced by Xml2Doc. + +The documentation states that for our current parsing use-cases it is unsafe to automatically _final() free any of the XmlNodes as they're a part of our XmlDoc tree. + +As such, we have threaded a tag reference through every dependent Pony instance so that it will be "impossible"™ for Xml2Doc._final() to execute the frees until all other dependent references are no longer reachable. diff --git a/libxml2/xml2doc.pony b/libxml2/xml2doc.pony index 1a65589..7a03541 100644 --- a/libxml2/xml2doc.pony +++ b/libxml2/xml2doc.pony @@ -57,7 +57,7 @@ class Xml2Doc LibXML2.xmlXPathRegisterNs(tmpctx, n, url) end let xptr: NullablePointer[XmlXPathObject] = LibXML2.xmlXPathEval(xpath, tmpctx) - let xpo: Xml2XPathResult = Xml2XPathObject(xptr) + let xpo: Xml2XPathResult = Xml2XPathObject(recover tag this end, xptr) LibXML2.xmlXPathFreeContext(tmpctx) xpo @@ -69,11 +69,7 @@ class Xml2Doc if the document has no root element or the returned pointer is null. """ let ptrx: NullablePointer[XmlNode] = LibXML2.xmlDocGetRootElement(ptr') - Xml2Node.fromPTR(ptrx)? + Xml2Node.fromPTR(recover tag this end, ptrx)? -/* - * DO NOT REENABLE THIS UNTIL WE'VE FOUND A WAY TO ENSURE THAT NO OTHER REFERENCES - * EXIST IN OUR PROGRAM. THIS HAS TO BE LAST OR ELSE SEGFAULT! - */ -// fun _final() => -// LibXML2.xmlFreeDoc(ptr') + fun _final() => + LibXML2.xmlFreeDoc(ptr') diff --git a/libxml2/xml2node.pony b/libxml2/xml2node.pony index a9827ad..1804e6c 100644 --- a/libxml2/xml2node.pony +++ b/libxml2/xml2node.pony @@ -8,8 +8,9 @@ class Xml2Node var ptr': NullablePointer[XmlNode] var ptr: XmlNode var allocated: Bool + var xml2doc: Xml2Doc tag - new fromPTR(ptrx: NullablePointer[XmlNode])? => + new fromPTR(xml2doc': Xml2Doc tag, ptrx: NullablePointer[XmlNode])? => """ Create an `Xml2Node` from a non-null libxml2 `xmlNode*`. @@ -19,6 +20,7 @@ class Xml2Node `ptr'`, assigns the underlying `XmlNode` to `ptr`, and marks the node as allocated. """ + xml2doc = xml2doc' if (ptrx.is_none()) then error else @@ -46,7 +48,7 @@ class Xml2Node end LibXML2.xmlXPathSetContextNode(ptr', tmpctx) let xptr: NullablePointer[XmlXPathObject] = LibXML2.xmlXPathEval(xpath, tmpctx) - let xpo: Xml2XPathResult = Xml2XPathObject(xptr) + let xpo: Xml2XPathResult = Xml2XPathObject(xml2doc, xptr) LibXML2.xmlXPathFreeContext(tmpctx) xpo @@ -142,11 +144,11 @@ class Xml2Node var fptr: NullablePointer[XmlNode] = LibXML2.xmlFirstElementChild(ptr') try - rv.push(Xml2Node.fromPTR(fptr)?) + rv.push(Xml2Node.fromPTR(xml2doc, fptr)?) while (elementCount > 0) do elementCount = elementCount - 1 fptr = LibXML2.xmlNextElementSibling(fptr) - rv.push(Xml2Node.fromPTR(fptr)?) + rv.push(Xml2Node.fromPTR(xml2doc, fptr)?) end end rv diff --git a/libxml2/xml2xpathobject.pony b/libxml2/xml2xpathobject.pony index 698477c..7957d50 100644 --- a/libxml2/xml2xpathobject.pony +++ b/libxml2/xml2xpathobject.pony @@ -11,7 +11,7 @@ primitive Xml2XPathObject Factory for converting a raw libxml2 `xmlXPathObject*` into a high-level `Xml2XPathResult` (nodes, boolean, number, or string) usable from Pony. """ - fun apply(ptrx: NullablePointer[XmlXPathObject]): Xml2XPathResult => + fun apply(xml2doc: Xml2Doc tag, ptrx: NullablePointer[XmlXPathObject]): Xml2XPathResult => """ Wrap a nullable `xmlXPathObject*` and return an `Xml2XPathResult` matching the XPath result type. @@ -49,7 +49,7 @@ primitive Xml2XPathObject nodeset.nodeTab, nodeset.nodeNr.usize()) for f in nodearray'.values() do - nodearray.push(Xml2Node.fromPTR(f)?) + nodearray.push(Xml2Node.fromPTR(xml2doc, f)?) end return nodearray end