Re: [NSXML + libxml2] external retains

[Doug Simons]

Hi Niels,

Thanks for your feedback and helpful suggestions! It's good to know that other 
people are looking at the code and trying it out.

I apologize for the current state of the code regarding memory management. It's 
actually a work in progress right now. I've been actively working on the 
problems you mentioned today and have already implemented the needed code for 
the detach method. Your proposed approach may be simpler though, so I'll give 
it some more thought before I check it in. The conditional you mentioned in the 
retain method was a little hack to temporarily try to avoid some crashes. As 
you noted, it's incorrect, and probably shouldn't have been checked in. Look 
for some more updates in the next day or two.

Regards,

Doug Simons

On Jan 26, 2012, at 4:05 PM, Niels Grewe wrote:

> Hi Doug, Greg and all,
> 
> I've just been digging into the new libxml2 powered NSXML code and got a
> bit confused about the code path that takes care of external references.
> I get that we want to increase the retain count of the top node in a
> tree if any object outside the tree makes reference to some node in the
> tree. Otherwise, we'd get dangling parent pointers when nothing
> references the top node and it gets deallocated.
> 
> But from looking at the code, I somehow think that we should also be
> decreasing the top node's external retain count when a node gets
> detached from the tree (which we don't do presently). But with the way
> things are done now, I don't see how we would do that because once the
> node is attached the record keeping for external references is taken
> over by the top node. The situation basically looks like this ("|"
> denotes the parent-child-relation where the parent is written above the
> child and "<-" denotes an external object holding a reference to the node):
> 
> A     (externalRetains == 2)
> |
> B<-X  (externalRetains == 0)
> |
> C     (externalRetains == 0)
> |
> D<-Y  (externalRetains == 0)
> 
> Now if we detach, say, C, then we have no way to tell:
> 
> a) that we should send C and extra -retain in order to prevent D from
> ending up with a dangling reference to C.
> 
> b) that we should decrease the externalRetains of A by 1. And we need to
> do that, because if we don't, and X stops retaining B, we start leaking
> A and B.
> 
> This would be much easier to handle if we were tracking the extra
> refcount all the way through the tree:
> 
> 
> A     (externalRetains == 2)
> |
> B<-X  (externalRetains == 2)
> |
> C     (externalRetains == 1)
> |
> D<-X  (externalRetains == 1)
> 
> Would that be about right? Also: Couldn't we just do away with this
> whole contraption if we had the children retain their parents while the
> parents only hold weak references to them?
> 
> Also there is another thing I cannot quite wrap my head around right now
> and it would be nice if someone could elaborate on that as well: Why
> does NSXMLNode's -release method decrease the externalRetains only "if
> ([self retainCount] == [internal->subNodes count])" (NSXMLNode.m:590)?
> 
> I already apologise if I'm just overlooking the obvious here. Also, I
> hope that I don't sound overly cirtical about this code, which I think
> is really a nice new feature. I already turned on some bits in DBusKit
> that require NSXML, and it turns out that it's working quite well in
> general :-)
> 
> Cheers,
> 
> Niels
>