1 xmlelement memory leak

Development
1

So I believe I have traced a XmlElement memory leak to the following code:

auto newMaster =  parseXML(masterCopy);  //XmlDocument::parse(masterCopy);
auto newProps = new XmlElement("props");
newProps->addChildElement(newMaster.get());

this seems to end up with 1 xmlelement memory link. I seem to see some references to a hanging element when setting to a child element.

Any assistance would be much appreciated.

thanks

Peter

2

So do you delete the allocated newProps? If not, this is your leak.

A memory leak is always some memory that has been allocated with new (or malloc) and has not been freed later with delete (or free). Instead of calling delete manually, in modern C++ code pointers to allocated memory are usually wrapped into a std::unique_ptr which manages the memory automatically by deleting the held instance in case it goes out of scope or is re-assigned with a new instance.

To do so, change your code to

auto newProps = std::make_unique<XmlElement> ("props");

now newProps is a std::unique_ptr<XmlElement> instead of a raw XmlElement* pointer as in your solution.

But beware, there is another problem in the code in the following line. newMaster obviously also is a std::unique_ptr<XmlElement> – meaning that it will automatically be deleted when it goes out of scope. But you add it as child element to new props. Looking at the documentation for XmlElement::addChildElement we read

Appends an element to this element’s list of children.

Child elements are deleted automatically when their parent is deleted, so make sure the object that you pass in will not be deleted by anything else, and make sure it’s not already the child of another element.

In your code, both, the unique pointer holding it and newProps will delete it at the end of their lifetime which will result in a crash. This probably was no problem until now since you didn’t delete newProps before, so it didn’t attempt to delete its child either. To prevent that, you explicitly have to instruct newMaster to no longer take care of deleting the instance it holds by releasing it like

newProps->addChildElement (newMaster.release());

Sidenote: The XmlElement class is quite old. A more modern API would probably directly take a unique ptr instead of a raw pointer so that you could move the unique pointer into its new owner like addChildElement (std::move (newMaster)). Some classes like e.g. the OwnedArray already allow this.

In case this was new to you, I’d highly recommend you to have an extensive read on C++ memory management, otherwise you’ll probably run into a lot of strange problems with your code sooner or later.

1 Like