Re: Unresolved Issues with libxml2

[Fred Kiefer]

Hi Doug,

thank you for taking the time to test the new XML implementation in detail.

On 23.03.2012 01:24, Doug Simons wrote:
> Hi Fred,
>
> I finally found some time to try the XML code with your recent
> changes. First, I appreciate all of the work you've done to finish up
> the parts of this code that we weren't able to complete in the
> initial implementation. Also, I can confirm that the change you made
> to implement prettyPrint seems to be working for me. The code you
> implemented for NSXMLNodeCompactEmptyElement is performing the right
> operation but in the opposite direction. So I reversed the logic and
> also added a test for the complementary NSXMLNodeExpandEmptyElement
> flag that (for some reason) Cocoa also defines. If neither flag is
> set, it defaults to the expanded form, which I believe is the default
> in Cocoa.

I think you are correct here. Could you please go ahead and commit your 
changes. I am not sure I did get all the details from your description.

> Some things seem broken, though. The first is that the change you
> made in -copyWithZone: (passing a value of 2 rather than 1 to the
> xmlCopyNode() function) stops it from copying the node's children. So
> I've changed that back to 1, which does a deep copy.

Once again you are right. This change was a bug. I did not understand 
the results of the different values and relied on the online 
documentation of libxml2, which doesn't really help here. I made that 
change in the code myself already.

> More serious is that I am getting a crash when dealloc'ing objects in
> some situations. I haven't fully tracked it down but I suspect it
> might be related to the code you added in the detach method that
> creates a new document and calls xmlDOMWrapAdoptNode(). I'm not sure
> what all of that does, but in debugging I noticed that I ended up
> with what looks to me like a malformed libxml node tree. I have a
> text node that has a parent and a document:
>
> (gdb) p *node $24 = {_private = 0x0, type = XML_TEXT_NODE, name =
> 0x4650d0 "text", children = 0x0, last = 0x0, parent = 0x88e52f8, next
> = 0x0, prev = 0x0, doc = 0x88e4ee0, ns = 0x0, content = 0x88e4f68
> "7", properties = 0x0, nsDef = 0x0, psvi = 0x0, line = 1, extra = 0}
>
> Its parent is an element with a document, but no parent:
>
> (gdb) p *node.parent $27 = {_private = 0x88e4e44, type =
> XML_ELEMENT_NODE, name = 0x88e4f58 "num", children = 0x88e5338, last
> = 0x88e5338, parent = 0x0, next = 0x0, prev = 0x0, doc = 0x88e4ee0,
> ns = 0x0, content = 0x0, properties = 0x0, nsDef = 0x0, psvi = 0x0,
> line = 0, extra = 0}
>
> And the document that both of them belong to has no children: (gdb) p
> *node.doc $26 = {_private = 0x88e1c94, type = XML_DOCUMENT_NODE, name
> = 0x0, children = 0x0, last = 0x0, parent = 0x0, next = 0x0, prev =
> 0x0, doc = 0x88e4ee0, compression = -1, standalone = -1, intSubset =
> 0x0, extSubset = 0x0, oldNs = 0x0, version = 0x88e4f48 "1.0",
> encoding = 0x0, ids = 0x0, refs = 0x0, URL = 0x0, charset = 1, dict =
> 0x0, psvi = 0x0, parseFlags = 0, properties = 32}
>
> This seems like a bad state of affairs. If nothing else, it means
> that the rootDocument method will return an object that has no
> reference to its children. And I suspect that somehow this is leading
> up to the crash which occurs when the NSXMLDocument is dealloc'ed:

I should explain why I am using this private documents on nodes and 
attributes (although I already send a partial explanation in a previous 
mail). These private documents are used for two purposes. The first on 
deals with strings while the second one has to do with namespaces.

libxml2 tries to optimise the memory usage of strings. In XML the same 
tags get used a lot and they save space by using the same memory for all 
the strings with the same value in a document. These strings get cached 
in a dictionary on the document and the nodes only reference these 
strings. This is all fine as long as a node is connected to its 
document, but when a node gets detached and maybe later added to a 
different document things get more complicated. After a call to 
xmlUnlinkNode the strings still point to the dictionary of the old 
document. When the document gets freed these strings would point to 
freed memory. To avoid that, I create a private document for the node 
and use the libxml2 function xmlDOMWrapAdoptNode to transfer these 
strings to the new document. (This also handles namespaces, which is 
just as important) When the node gets reattached to a new document a 
similar process happens and the private document gets freed. This all 
requires a libxml2 version > 2.6.20, for older ones we are in deep 
trouble and the call to xmlSetTreeDoc that I added for this case wont 
really help.
What I should change, as you noted and as a FIXME in the code states, is 
that we should not return these private documents in any way. I just 
change the code to no longer return the document, when the parent is NULL.
Now you may ask yourself, how all this could ever work with the old 
implementation. Good question! It just didn't, it seemed to work, but 
only when all the documents would stay around long enough to be cleaned 
up after all the nodes. That is, you were lucky and a lot of libxml2 
memory got leaked. (But leaking memory is still better than crashing as 
we do now)

The second purpose these private documents get used for is to store 
unresolved namespaces. Again this gets done to make sure we don't have 
any memory corruption. I really need to explain the concept here in more 
detail, but I already know that you are not interested in namespaces and 
all this should not be relevant in this specific case.

> Program received signal SIGSEGV, Segmentation fault. 0x005db761 in
> free () from /lib/tls/i686/cmov/libc.so.6 (gdb) bt 9 #0  0x005db761
> in free () from /lib/tls/i686/cmov/libc.so.6 #1  0x003a61f8 in
> xmlFreeNode () from /usr/lib/libxml2.so.2 #2  0x00944a11 in
> -[NSXMLNode dealloc] (self=0x88e1c94, _cmd=0xb3c9d0) at
> NSXMLNode.m:1192 #3  0x0093be62 in -[NSXMLDocument dealloc]
> (self=0x88e1c94, _cmd=0xb0e6e8) at NSXMLDocument.m:54 #4  0x008a2cc4
> in -[NSObject release] (self=0x88e1c94, _cmd=0xadd8d0) at
> NSObject.m:2049 #5  0x007ea77f in -[NSAutoreleasePool emptyPool]
> (self=0x881a314, _cmd=0xadd8c0) at NSAutoreleasePool.m:656 #6
> 0x007ea5a5 in -[NSAutoreleasePool dealloc] (self=0x881a314,
> _cmd=0xadd8b8) at NSAutoreleasePool.m:538 #7  0x007ea0c8 in
> -[NSAutoreleasePool release] (self=0x881a314, _cmd=0x2dfa30) at
> NSAutoreleasePool.m:531 #8  0x0020d699 in endFrame (self=0x835b18c)
> at STRuntime.m:462 (More stack frames follow...)

Here an NSXMLDocument gets freed and this crashes in libxml2. Bad, we 
really need to fix this.

> I may be wrong about what is causing this -- a quick attempt to
> remove the code I was suspicious of in the detach method didn't solve
> the problem. I'm afraid I can't take any more time with this right
> now, but I can tell you that this occurs after approximately this
> sequence of steps (it's hard for me to pin down the exact steps
> because this is all invoked by some higher-level code, but I think
> this is very close):

Now having a text description of test code is quite nice, but you do 
understand that having it in code would actually help more?
You state that you don't have any more time to spend on this, but what 
you do is using up my time instead.

> I create 2 NSXMLDocuments by calling [NSXMLDocument
> initWithXMLString:options:error:] with the strings "<num>6</num>"
> and"<num>7</num>"
>
> In each of these Documents my code calls -rootElement to get the root
> element and then calls -detach on the element to remove it from its
> document. Then I believe -copyWithZone: is called on the root
> elements, and these copies are what are used by the rest of the
> code.
>
> Then, in the first element, the code changes the contents from "6" to
> "7" by calling -setStringValue: on the text node.
>
> Then the code compares the two detached element trees. All of that
> works fine until it releases the pool and crashes.

I wrote some test code that does what you describe (Now in SVN with the 
name transfer.m):

#import "ObjectTesting.h"
#import <Foundation/NSAutoreleasePool.h>
#import <Foundation/NSXMLDocument.h>
#import <Foundation/NSXMLElement.h>

int main()
{
  NSAutoreleasePool *arp = [NSAutoreleasePool new];
  NSXMLElement *elem1 = [[NSXMLElement alloc] initWithXMLString: 
@"<num>6</num>" error: NULL];
  NSXMLElement *elem2 = [[NSXMLElement alloc] initWithXMLString: 
@"<num>7</num>" error: NULL];
  NSXMLElement *copy1 = [elem1 copy];
  NSXMLElement *copy2 = [elem2 copy];

  [copy1 setStringValue: @"7"];
  PASS_EQUAL(copy1, copy2, "equal after setStringValue:");

  [arp drain];
  arp = nil;

  return 0;
}

This code works fine, and did so even before I made the changes listed 
above. (OK, I had to correct the value for the deep copy)
The only difference to your description is the usage of 
initWithXMLString:error:, but the implementation of that method is just 
what you describe:

- (id) initWithXMLString: (NSString*)string
                   error: (NSError**)error
{
  NSXMLElement *result = nil;
  NSXMLDocument *tempDoc =
    [[NSXMLDocument alloc] initWithXMLString: string
                                     options: 0
                                       error: error];
  if (tempDoc != nil)
    {
      result = RETAIN([tempDoc rootElement]);
      [result detach]; // detach from document.
    }
  [tempDoc release];
  [self release];

  return result;
}

I would say that this means that the problem comes from your higher 
level code. Could it be that this code access the rootDocument of the 
standalone nodes and tries to free up that? This problem should be fixed 
by the changes I just made. Please try again and if there are any 
remaining issue, PLEASE provide actual test code.
You see, you are getting paid for what you do here. I am not. This 
clearly means that you should try to make my work as easy as possible.

Cheers
Fred

> This same sequence of steps was working without crashing with the XML
> code as it existed a couple weeks ago. So I'm hoping maybe you have a
> clue about which changes might lead to this.
>
> Thanks,
>
> Doug
>
>
>
>
> On Mar 14, 2012, at 4:27 PM, Fred Kiefer wrote:
>
> > On 29.02.2012 23:28, Doug Simons wrote:
> > > Since we've submitted the new implementation of the NSXML...
> > > classes based on libxml2 and people are beginning to use them, I
> > > thought I would mention some remaining unresolved issues in the
> > > hope that other people might have more experience with the
> > > libxml2 libraries and have some ideas about how to solve them.
> > > These are currently the top issues on my list:
> > >
> > > 1. Parsing an XML document generates text nodes in the tree for
> > > whitespace between elements even when the XML_PARSE_NOBLANKS
> > > option is given.  Cocoa doesn't do this.
> > >
> > > 2. Find a way to control formatting of "empty" nodes. Cocoa has
> > > the options NSXMLNodeExpandEmptyElement and
> > > NSXMLNodeCompactEmptyElement to control whether an empty node foo
> > > is displayed as "<foo></foo>" or as"<foo/>" . Currently our
> > > libxml2 implementation will only display the latter.
> > >
> > > 3. Find a way to control "pretty-print" formatting. Cocoa has an
> > > option NSXMLNodePrettyPrint to control whether a string
> > > representation of a tree will include indentation for enhanced
> > > readability or not. Currently our libxml2 implementation always
> > > includes indentation.
> > >
> > > I have searched the libxml2 documentation for ways to control
> > > these issues but haven't come up with anything that works (in
> > > particular I tried the xmlKeepBlanksDefault() function for issue
> > > 1 without any success).
> >
> > I hope that I have solved the last two of your issues. Could you
> > please give it a try?
> >
> > _______________________________________________ Gnustep-dev mailing
> > list address@hidden
> > https://lists.gnu.org/mailman/listinfo/gnustep-dev