Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions .release-notes/2.md
Original file line number Diff line number Diff line change
@@ -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.
12 changes: 4 additions & 8 deletions libxml2/xml2doc.pony
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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')
10 changes: 6 additions & 4 deletions libxml2/xml2node.pony
Original file line number Diff line number Diff line change
Expand Up @@ -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*`.

Expand All @@ -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
Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions libxml2/xml2xpathobject.pony
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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
Expand Down