Permalink
Browse files

Greatly improving thread-safety. No more having to worry about not re…

…leasing the root element when you have references to child elements.
  • Loading branch information...
1 parent d03d9eb commit 5a689e01602bef1fa4e62e75b5b733ec15e605e3 @robbiehanson committed Jul 29, 2011
Showing with 299 additions and 214 deletions.
  1. +86 −8 KissXML/DDXML.h
  2. +10 −10 KissXML/DDXMLDocument.m
  3. +41 −32 KissXML/DDXMLElement.m
  4. +5 −2 KissXML/DDXMLNode.h
  5. +121 −136 KissXML/DDXMLNode.m
  6. +13 −13 KissXML/Private/DDXMLPrivate.h
  7. +23 −13 UnitTesting/DDXMLTesting.m
View
@@ -3,15 +3,93 @@
#import "DDXMLDocument.h"
-// KissXML has rather straight-forward memory management.
-// However, if the rules are not followed,
-// it is often difficult to track down the culprit.
-//
-// Enabling this option will help you track down the orphaned subelement.
-// More information can be found on the wiki page:
+// KissXML has rather straight-forward memory management:
// http://code.google.com/p/kissxml/wiki/MemoryManagementThreadSafety
//
-// Please keep in mind that this option is for debugging only.
-// It significantly slows down the library, and should NOT be enabled for production builds.
+// There are 3 important concepts to keep in mind when working with KissXML:
+//
+//
+// 1.) KissXML provides a light-weight wrapper around libxml.
+//
+// The parsing, creation, storage, etc of the xml tree is all done via libxml.
+// This is a fast low-level C library that's been around for ages, and comes pre-installed on Mac OS X and iOS.
+// KissXML provides an easy-to-use Objective-C library atop libxml.
+// So a DDXMLNode, DDXMLElement, or DDXMLDocument are simply objective-c objects
+// with pointers to the underlying libxml C structure.
+// Then only time you need to be aware of any of this is when it comes to equality.
+// In order to maximize speed and provide read-access thread-safety,
+// the library may create multiple DDXML wrapper objects that point to the same underlying xml node.
+// So don't assume you can test for equality with "==".
+// Instead use the isEqual method (as you should generally do with objects anyway).
+//
+//
+// 2.) XML is implicitly a tree heirarchy, and the XML API's are designed to allow traversal up & down the tree.
+//
+// The tree heirarchy and API contract have an implicit impact concerning memory management.
+//
+// <starbucks>
+// <latte/>
+// </starbucks>
+//
+// Imagine you have a DDXMLNode corresponding to the starbucks node,
+// and you have a DDXMLNode corresponding to the latte node.
+// Now imagine you release the starbucks node, but you retain a reference to the latte node.
+// What happens?
+// Well the latte node is a part of the xml tree heirarchy.
+// So if the latte node is still around, the xml tree heirarchy must stick around as well.
+// So even though the DDXMLNode corresponding to the starbucks node may get deallocated,
+// the underlying xml tree structure won't be freed until the latte node gets dealloacated.
+//
+// In general, this means that KissXML remains thread-safe when reading and processing a tree.
+// If you traverse a tree and fork off asynchronous tasks to process subnodes,
+// the tree will remain properly in place until all your asynchronous tasks have completed.
+// In other words, it just works.
//
+// However, if you parse a huge document into memory, and retain a single node from the giant xml tree...
+// Well you should see the problem this creates.
+// Instead, in this situation, copy or detach the node if you want to keep it around.
+// Or just extract the info you need from it.
+//
+//
+// 3.) KissXML is read-access thread-safe, but write-access thread-unsafe (designed for speed).
+//
+// <starbucks>
+// <latte/>
+// </starbucks>
+//
+// Imagine you have a DDXMLNode corresponding to the starbucks node,
+// and you have a DDXMLNode corresponding to the latte node.
+// What happens if you invoke [starbucks removeChildAtIndex:0]?
+// Well the undelying xml tree will remove the latte node, and release the associated memory.
+// And what if you still have a reference to the DDXMLNode that corresponds to the latte node?
+// Well the short answer is that you shouldn't use it. At all.
+// This is pretty obvious when you think about it from the context of this simple example.
+// But in the real world, you might have multiple threads running in parallel,
+// and you might accidently modify a node while another thread is processing it.
+//
+// To completely fix this problem, and provide write-access thread-safety, would require extensive overhead.
+// This overhead is completely unwanted in the majority of cases.
+// Most XML usage patterns are heavily read-only.
+// And in the case of xml creation or modification, it is generally done on the same thread.
+// Thus the KissXML library is write-access thread-unsafe, but provides speedier performance.
+//
+// However, when such a bug does creep up, it produces horrible side-effects.
+// Essentially the pointer to the underlying xml structure becomes a dangling pointer,
+// which means that accessing the dangling pointer might give you the correct results, or completely random results.
+// And attempting to make modifications to non-existant xml nodes via the dangling pointer might do nothing,
+// or completely corrupt your heap and cause un-explainable crashes in random parts of your library.
+// Heap corruption is one of the worst problems to track down.
+// So to help out, the library provides a debugging macro to track down these problems.
+// That is, if you invalidate the write-access thread-unsafe rule,
+// this macro will tell you when you're trying to access a no-dangling pointer.
+//
+// How does it work?
+// Well everytime a DDXML wrapper object is created atop a libxml structure,
+// it marks the linkage in a table.
+// And everytime a libxml structure is freed, it destorys all corresponding linkages in the table.
+// So everytime a DDXML wrapper objects is about to dereference it's pointer,
+// it first ensures the linkage still exists in the table.
+//
+// The debugging macro adds a significant amount of overhead, and shouldn't be enabled on production builds.
+
#define DDXML_DEBUG_MEMORY_ISSUES 0
@@ -8,29 +8,29 @@ @implementation DDXMLDocument
* Returns a DDXML wrapper object for the given primitive node.
* The given node MUST be non-NULL and of the proper type.
**/
-+ (id)nodeWithDocPrimitive:(xmlDocPtr)doc freeOnDealloc:(BOOL)flag
++ (id)nodeWithDocPrimitive:(xmlDocPtr)doc owner:(DDXMLNode *)owner
{
- return [[[DDXMLDocument alloc] initWithDocPrimitive:doc freeOnDealloc:flag] autorelease];
+ return [[[DDXMLDocument alloc] initWithDocPrimitive:doc owner:owner] autorelease];
}
-- (id)initWithDocPrimitive:(xmlDocPtr)doc freeOnDealloc:(BOOL)flag
+- (id)initWithDocPrimitive:(xmlDocPtr)doc owner:(DDXMLNode *)inOwner
{
- self = [super initWithPrimitive:(xmlKindPtr)doc freeOnDealloc:flag];
+ self = [super initWithPrimitive:(xmlKindPtr)doc owner:inOwner];
return self;
}
-+ (id)nodeWithPrimitive:(xmlKindPtr)kindPtr freeOnDealloc:(BOOL)flag
++ (id)nodeWithPrimitive:(xmlKindPtr)kindPtr owner:(DDXMLNode *)owner
{
// Promote initializers which use proper parameter types to enable compiler to catch more mistakes
- NSAssert(NO, @"Use nodeWithDocPrimitive:freeOnDealloc:");
+ NSAssert(NO, @"Use nodeWithDocPrimitive:owner:");
return nil;
}
-- (id)initWithPrimitive:(xmlKindPtr)kindPtr freeOnDealloc:(BOOL)flag
+- (id)initWithPrimitive:(xmlKindPtr)kindPtr owner:(DDXMLNode *)inOwner
{
// Promote initializers which use proper parameter types to enable compiler to catch more mistakes.
- NSAssert(NO, @"Use initWithDocPrimitive:freeOnDealloc:");
+ NSAssert(NO, @"Use initWithDocPrimitive:owner:");
[self release];
return nil;
@@ -81,7 +81,7 @@ - (id)initWithData:(NSData *)data options:(NSUInteger)mask error:(NSError **)err
return nil;
}
- return [self initWithDocPrimitive:doc freeOnDealloc:YES];
+ return [self initWithDocPrimitive:doc owner:nil];
}
/**
@@ -100,7 +100,7 @@ - (DDXMLElement *)rootElement
xmlNodePtr rootNode = xmlDocGetRootElement(doc);
if (rootNode != NULL)
- return [DDXMLElement nodeWithElementPrimitive:rootNode freeOnDealloc:NO];
+ return [DDXMLElement nodeWithElementPrimitive:rootNode owner:self];
else
return nil;
}
Oops, something went wrong. Retry.

0 comments on commit 5a689e0

Please sign in to comment.