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?