Skip to content

Commit 5a689e0

Browse files
committed
Greatly improving thread-safety. No more having to worry about not releasing the root element when you have references to child elements.
1 parent d03d9eb commit 5a689e0

File tree

7 files changed

+299
-214
lines changed

7 files changed

+299
-214
lines changed

KissXML/DDXML.h

Lines changed: 86 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -3,15 +3,93 @@
33
#import "DDXMLDocument.h"
44

55

6-
// KissXML has rather straight-forward memory management.
7-
// However, if the rules are not followed,
8-
// it is often difficult to track down the culprit.
9-
//
10-
// Enabling this option will help you track down the orphaned subelement.
11-
// More information can be found on the wiki page:
6+
// KissXML has rather straight-forward memory management:
127
// http://code.google.com/p/kissxml/wiki/MemoryManagementThreadSafety
138
//
14-
// Please keep in mind that this option is for debugging only.
15-
// It significantly slows down the library, and should NOT be enabled for production builds.
9+
// There are 3 important concepts to keep in mind when working with KissXML:
10+
//
11+
//
12+
// 1.) KissXML provides a light-weight wrapper around libxml.
13+
//
14+
// The parsing, creation, storage, etc of the xml tree is all done via libxml.
15+
// This is a fast low-level C library that's been around for ages, and comes pre-installed on Mac OS X and iOS.
16+
// KissXML provides an easy-to-use Objective-C library atop libxml.
17+
// So a DDXMLNode, DDXMLElement, or DDXMLDocument are simply objective-c objects
18+
// with pointers to the underlying libxml C structure.
19+
// Then only time you need to be aware of any of this is when it comes to equality.
20+
// In order to maximize speed and provide read-access thread-safety,
21+
// the library may create multiple DDXML wrapper objects that point to the same underlying xml node.
22+
// So don't assume you can test for equality with "==".
23+
// Instead use the isEqual method (as you should generally do with objects anyway).
24+
//
25+
//
26+
// 2.) XML is implicitly a tree heirarchy, and the XML API's are designed to allow traversal up & down the tree.
27+
//
28+
// The tree heirarchy and API contract have an implicit impact concerning memory management.
29+
//
30+
// <starbucks>
31+
// <latte/>
32+
// </starbucks>
33+
//
34+
// Imagine you have a DDXMLNode corresponding to the starbucks node,
35+
// and you have a DDXMLNode corresponding to the latte node.
36+
// Now imagine you release the starbucks node, but you retain a reference to the latte node.
37+
// What happens?
38+
// Well the latte node is a part of the xml tree heirarchy.
39+
// So if the latte node is still around, the xml tree heirarchy must stick around as well.
40+
// So even though the DDXMLNode corresponding to the starbucks node may get deallocated,
41+
// the underlying xml tree structure won't be freed until the latte node gets dealloacated.
42+
//
43+
// In general, this means that KissXML remains thread-safe when reading and processing a tree.
44+
// If you traverse a tree and fork off asynchronous tasks to process subnodes,
45+
// the tree will remain properly in place until all your asynchronous tasks have completed.
46+
// In other words, it just works.
1647
//
48+
// However, if you parse a huge document into memory, and retain a single node from the giant xml tree...
49+
// Well you should see the problem this creates.
50+
// Instead, in this situation, copy or detach the node if you want to keep it around.
51+
// Or just extract the info you need from it.
52+
//
53+
//
54+
// 3.) KissXML is read-access thread-safe, but write-access thread-unsafe (designed for speed).
55+
//
56+
// <starbucks>
57+
// <latte/>
58+
// </starbucks>
59+
//
60+
// Imagine you have a DDXMLNode corresponding to the starbucks node,
61+
// and you have a DDXMLNode corresponding to the latte node.
62+
// What happens if you invoke [starbucks removeChildAtIndex:0]?
63+
// Well the undelying xml tree will remove the latte node, and release the associated memory.
64+
// And what if you still have a reference to the DDXMLNode that corresponds to the latte node?
65+
// Well the short answer is that you shouldn't use it. At all.
66+
// This is pretty obvious when you think about it from the context of this simple example.
67+
// But in the real world, you might have multiple threads running in parallel,
68+
// and you might accidently modify a node while another thread is processing it.
69+
//
70+
// To completely fix this problem, and provide write-access thread-safety, would require extensive overhead.
71+
// This overhead is completely unwanted in the majority of cases.
72+
// Most XML usage patterns are heavily read-only.
73+
// And in the case of xml creation or modification, it is generally done on the same thread.
74+
// Thus the KissXML library is write-access thread-unsafe, but provides speedier performance.
75+
//
76+
// However, when such a bug does creep up, it produces horrible side-effects.
77+
// Essentially the pointer to the underlying xml structure becomes a dangling pointer,
78+
// which means that accessing the dangling pointer might give you the correct results, or completely random results.
79+
// And attempting to make modifications to non-existant xml nodes via the dangling pointer might do nothing,
80+
// or completely corrupt your heap and cause un-explainable crashes in random parts of your library.
81+
// Heap corruption is one of the worst problems to track down.
82+
// So to help out, the library provides a debugging macro to track down these problems.
83+
// That is, if you invalidate the write-access thread-unsafe rule,
84+
// this macro will tell you when you're trying to access a no-dangling pointer.
85+
//
86+
// How does it work?
87+
// Well everytime a DDXML wrapper object is created atop a libxml structure,
88+
// it marks the linkage in a table.
89+
// And everytime a libxml structure is freed, it destorys all corresponding linkages in the table.
90+
// So everytime a DDXML wrapper objects is about to dereference it's pointer,
91+
// it first ensures the linkage still exists in the table.
92+
//
93+
// The debugging macro adds a significant amount of overhead, and shouldn't be enabled on production builds.
94+
1795
#define DDXML_DEBUG_MEMORY_ISSUES 0

KissXML/DDXMLDocument.m

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -8,29 +8,29 @@ @implementation DDXMLDocument
88
* Returns a DDXML wrapper object for the given primitive node.
99
* The given node MUST be non-NULL and of the proper type.
1010
**/
11-
+ (id)nodeWithDocPrimitive:(xmlDocPtr)doc freeOnDealloc:(BOOL)flag
11+
+ (id)nodeWithDocPrimitive:(xmlDocPtr)doc owner:(DDXMLNode *)owner
1212
{
13-
return [[[DDXMLDocument alloc] initWithDocPrimitive:doc freeOnDealloc:flag] autorelease];
13+
return [[[DDXMLDocument alloc] initWithDocPrimitive:doc owner:owner] autorelease];
1414
}
1515

16-
- (id)initWithDocPrimitive:(xmlDocPtr)doc freeOnDealloc:(BOOL)flag
16+
- (id)initWithDocPrimitive:(xmlDocPtr)doc owner:(DDXMLNode *)inOwner
1717
{
18-
self = [super initWithPrimitive:(xmlKindPtr)doc freeOnDealloc:flag];
18+
self = [super initWithPrimitive:(xmlKindPtr)doc owner:inOwner];
1919
return self;
2020
}
2121

22-
+ (id)nodeWithPrimitive:(xmlKindPtr)kindPtr freeOnDealloc:(BOOL)flag
22+
+ (id)nodeWithPrimitive:(xmlKindPtr)kindPtr owner:(DDXMLNode *)owner
2323
{
2424
// Promote initializers which use proper parameter types to enable compiler to catch more mistakes
25-
NSAssert(NO, @"Use nodeWithDocPrimitive:freeOnDealloc:");
25+
NSAssert(NO, @"Use nodeWithDocPrimitive:owner:");
2626

2727
return nil;
2828
}
2929

30-
- (id)initWithPrimitive:(xmlKindPtr)kindPtr freeOnDealloc:(BOOL)flag
30+
- (id)initWithPrimitive:(xmlKindPtr)kindPtr owner:(DDXMLNode *)inOwner
3131
{
3232
// Promote initializers which use proper parameter types to enable compiler to catch more mistakes.
33-
NSAssert(NO, @"Use initWithDocPrimitive:freeOnDealloc:");
33+
NSAssert(NO, @"Use initWithDocPrimitive:owner:");
3434

3535
[self release];
3636
return nil;
@@ -81,7 +81,7 @@ - (id)initWithData:(NSData *)data options:(NSUInteger)mask error:(NSError **)err
8181
return nil;
8282
}
8383

84-
return [self initWithDocPrimitive:doc freeOnDealloc:YES];
84+
return [self initWithDocPrimitive:doc owner:nil];
8585
}
8686

8787
/**
@@ -100,7 +100,7 @@ - (DDXMLElement *)rootElement
100100
xmlNodePtr rootNode = xmlDocGetRootElement(doc);
101101

102102
if (rootNode != NULL)
103-
return [DDXMLElement nodeWithElementPrimitive:rootNode freeOnDealloc:NO];
103+
return [DDXMLElement nodeWithElementPrimitive:rootNode owner:self];
104104
else
105105
return nil;
106106
}

0 commit comments

Comments
 (0)