Re: Unresolved Issues with libxml2

[Fred Kiefer]

On 01.03.2012 11:08, Richard Frith-Macdonald wrote:
> On 1 Mar 2012, at 09:43, Fred Kiefer wrote:
>
> > On 01.03.2012 07:34, Richard Frith-Macdonald wrote:
> > > But these seem rather cosmetic issues ... to my mind the place
> > > that really needs work is defining an object ownership model and
> > > implementing memory management correctly.  I recall that this
> > > was about the hardest part of writing the original XML DOM
> > > support for gnustep-base several years ago, and it's something we
> > > need to get sorted out for NSXMLNode before people start using it
> > > seriously.
> >
> > I fully agree here. Although I am not sure I like the old GSXML
> > implementation. To me it seems that there we get a new document
> > each time we ask a node for its document. And the same seems to be
> > true for all the tree walking methods. Now what happens if a user
> > retains such an object? Shouldn't it now keep its tree alive?
>
> Yes, the GSXML classes are a totally different design and intended to
> be used in a somewhat different way (and I don't think they ever got
> memory management right ... just 'good enough' to be able to avoid
> leaking).
>
> > What we need here is more test cases.
>
> Agreed.  In particular we need a lot of tests to run on Apple systems
> to find out exactly how their object ownership model works.  What
> happens when you create documents and then release them while holding
> references to various nodes within them?  Does behaviour vary
> depending on how documents are created?  What about standalone nodes
> without documents?  What about namespaces?!!

It turns out that things are a lot simpler than we thought. I wrote a 
few tests on Cocoa and nothing of that mysterious behaviour that we 
tried to mimic showed up. It looks like Apple implemented a simple top 
down approach. The parent retains the child and nothing more. And we had 
even test code that took advantage of that behaviour, see basic.m in 
NSXMLNode:

    NSXMLElement *node = [[NSXMLElement alloc] initWithKind: 
NSXMLElementKind];
    NSXMLDocument *docA = [[NSXMLDocument alloc] initWithRootElement: 
node];
    NSXMLDocument *docB = nil;
    // NSLog(@"Here...");
    [node detach];
    PASS(docB = [[NSXMLDocument alloc] initWithRootElement: node], 
"Detached children can be reattached.");
    [docA release];
    // NSLog(@"Here... again");
    [docB release]; <-- HERE
    docA = [[NSXMLDocument alloc] initWithRootElement: node];
    // NSLog(@"Yet again");
    PASS_EXCEPTION(docB = [[NSXMLDocument alloc] initWithRootElement: 
node], NSInternalInconsistencyException, "Reusing a child throws an 
exception");

Did you spot the release on the document docB? This make the node usable 
again, that is node no longer has a parent and may be used as a root 
element in docA. I added a few tests that prove this behaviour. After 
the release of docB parent of node is nil!
This could be implemented rather easily by detaching all subnodes of any 
deallocated node. The only clever bit needed here is to make sure we 
free up the libxml2 nodes that don't have corresponding Objective-C 
objects, but I think that will happen automatically if we free the 
libxml2 node after detaching all the subnodes. There may be a few cases 
where this behaviour is slightly slower, but it is a lot more consistent 
than the current code.
Any objections? Otherwise I go ahead and implement this.