Skip to content

Fix: libxml manual memory management #15906

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
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
45 changes: 38 additions & 7 deletions src/xml/attributes.cr
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,13 @@ require "./node"
class XML::Attributes
include Enumerable(Node)

# :nodoc:
def initialize(@node : Node)
end

def empty? : Bool
return true unless @node.element?

props = self.props
props.null?
end

Expand Down Expand Up @@ -39,23 +39,54 @@ class XML::Attributes
end

def []=(name : String, value)
if prop = find_prop(name)
# manually unlink the prop's children if we have live references, so
# xmlSetProp won't free them immediately
@node.document.unlink_cached_children(prop)
end

LibXML.xmlSetProp(@node, name, value.to_s)
value
end

def delete(name : String) : String?
value = self[name]?.try &.content
res = LibXML.xmlUnsetProp(@node, name)
value if res == 0
prop = find_prop(name)
return unless prop

value = ""
if content = LibXML.xmlNodeGetContent(prop)
value = String.new(content)
end

if node = @node.document.cached?(prop)
# can't call xmlUnsetProp: it would free the node
node.unlink
value
else
# manually unlink the prop's children if we have live references, so
# xmlUnsetProp won't free them immediately
@node.document.unlink_cached_children(prop)
value if LibXML.xmlUnsetProp(@node, name) == 0
end
end

private def find_prop(name)
prop = @node.to_unsafe.value.properties.as(LibXML::Node*)
while prop
if String.new(prop.value.name) == name
return prop
end
prop = prop.value.next
end
end

def each(&) : Nil
return unless @node.element?

props = self.props
until props.null?
yield Node.new(props)
props = props.value.next
yield Node.new(props.as(LibXML::Node*), @node.document)
props = props.value.next.as(LibXML::Attr*)
end
end

Expand All @@ -73,7 +104,7 @@ class XML::Attributes
pp.list("[", self, "]")
end

protected def props
protected def props : LibXML::Attr*
@node.to_unsafe.value.properties
end
end
5 changes: 5 additions & 0 deletions src/xml/builder.cr
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,11 @@ class XML::Builder
@writer = LibXML.xmlNewTextWriter(buffer)
end

# :nodoc:
def finalize
LibXML.xmlFreeTextWriter(@writer)
end

# Emits the start of the document.
def start_document(version = nil, encoding = nil) : Nil
call StartDocument, string_to_unsafe(version), string_to_unsafe(encoding), nil
Expand Down
21 changes: 9 additions & 12 deletions src/xml/libxml2.cr
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,7 @@ lib LibXML
fun xmlTextReaderReadOuterXml(reader : XMLTextReader) : UInt8*
fun xmlTextReaderExpand(reader : XMLTextReader) : Node*
fun xmlTextReaderCurrentNode(reader : XMLTextReader) : Node*
fun xmlTextReaderCurrentDoc(reader : XMLTextReader) : Doc*

fun xmlTextReaderSetErrorHandler(reader : XMLTextReader, f : TextReaderErrorFunc) : Void
fun xmlTextReaderSetStructuredErrorHandler(reader : XMLTextReader, f : StructuredErrorFunc, arg : Void*) : Void
Expand Down Expand Up @@ -372,18 +373,14 @@ lib LibXML
{% if compare_versions(LibXML::VERSION, "2.14.0") >= 0 %}
fun xmlSaveSetIndentString(SaveCtxPtr, UInt8*)
{% end %}

fun xmlFreeDoc(Doc*)
fun xmlFreeNode(Node*)
fun xmlFreeTextReader(XMLTextReader)
fun xmlFreeTextWriter(TextWriter)
fun xmlXPathFreeContext(XPathContext*)
fun xmlXPathFreeNodeSet(NodeSet*)
fun xmlXPathFreeObject(XPathObject*)
end

LibXML.xmlInitParser

LibXML.xmlMemSetup(
->GC.free,
->GC.malloc(LibC::SizeT),
->GC.realloc(Void*, LibC::SizeT),
->(str) {
len = LibC.strlen(str) + 1
copy = Pointer(UInt8).malloc(len)
copy.copy_from(str, len)
copy
}
)
1 change: 1 addition & 0 deletions src/xml/namespace.cr
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
class XML::Namespace
getter document : Node

# :nodoc:
def initialize(@document : Node, @ns : LibXML::NS*)
end

Expand Down
Loading