Cvs Commit: cvsnt

CVSROOT:	/usr/local/cvs
Module name:	cvsnt
Changes by:	[email protected]		Wed Nov 14 23:40:12 2007
On host:	mail.march-hare.com

Directory: cvsnt/cvsapi

M XmlNode.cpp		1.9 -> 1.10	Bug Id: 
M XmlNode.h		1.6 -> 1.7	Bug Id: 
M XmlTree.cpp		1.9 -> 1.10	Bug Id: 
M XmlTree.h		1.4 -> 1.5	Bug Id: 

Directory: cvsnt/cvsio

M CvsEntry.cpp		1.27 -> 1.28	Bug Id: 
M CvsEntry.h		1.15 -> 1.16	Bug Id: 

Directory: cvsnt/evsfs

M VirtualAccess.cpp		1.13 -> 1.14	Bug Id: 
M VirtualFs.cpp		1.41 -> 1.42	Bug Id: 
M VirtualFs.h		1.21 -> 1.22	Bug Id: 
M VirtualUser.cpp		1.15 -> 1.16	Bug Id: 

Directory: cvsnt/src

M server.cpp		1.220 -> 1.221	Bug Id: 

Log message:
OK - this is another attempt to fix the same problems I was 
fixing on 12 Nov 2007.  I previously changed the way 
nodes are free'd in cvsapi/xmlnode.cpp removing the call
to xmlFreeNodeList() since that seemed to crash a rather lot.

That really only moved the problem a little, but these latest
changes actually seem to fix things.  I will now attempt to
explain why..

Firstly:
EVS CXmlNode's do not have a one to one relationship to 
the the gnome libxml2 "nodes".

It is OK if there is a gnome "node" but no CXmlNode, but 
sometimes the same gnome "node" has several CXmlNodes - 
now referencing these after the associated tree has been 
deleted causes crashes.  

I've attempted to fix this by deleting the gnome node once 
the CXmlNode is deleted, but that is showing up just how 
widespread the "problem" is - the same node is duplicated
all over - now causing NULL references...

Secondly:
An EntryList contains static and non-static member 
functions (methods).  In particular "open" is static and
contains the a static (protected) variable which tracks
which entrieslists have been opened (added to the list).

However when a (non-static) entrylist is deleted/destroyed
the (static) list of entrylists is not updated.  This 
results in the next (open) call to open the same "entrylist"
(really a directory, like c:/temp/cvs12456) will not 
create (instantiate) a new entrylist but return a pointer
from the static (protected) list - which is invalid.

Deleting (static) entries from lists doesn't seem possible
from the ~ handler so I've had to call it from the entrylist
close() function.

Thirdly (and finally):
Deleting/Erasing/Clearing the entmap of CCvsEntryList appears
to be a lot more difficult than it should be.   The call to
entmap.clear() in the static method CCvsEntryList::flush() 
regularly crashed (particularly client side or local).  

The clear is rather complex, because the entmap has an
evs::filename and an EntryListPtr.  When the entmap is
cleared and it has a valid EntryList pointer the 
CCvsEntryList::close() method is called from the 
~CCvsEntryList (indicating that entmap.clear in fact deletes
instantiated classes) and the EntryList is destroyed.  

Remember that my second point above is that the close did
not delete the entrylist pointer from the entmap, so now
we've got a catch-22.  If we call entrylist->close then 
the entmap "pointer" needs to be erased, but if we call 
entmap->clear then we need to entrylist->close().

Whenever there are multiple pointers to a single object
any change to that object must result in the nullification
of all the pointers to it - the easiest way is to only have
the pointer in one place.  In this case that is not 
possible.

However (conveniently) the m_fulldir I added to EntryList
to solve problem 2 above can be used here as well.  If 
clear() is called, "all" that needs to happen is that the
m_fulldir for each item in the list is set to NULL before
the call to clear().  It'd be just as easy to erase() each
individually...

Summary:
With this combination of tighter control of CXmlNode's, 
maps and EntryLists the "bugs" seem fixed.  

However it could be that the actual crashes I was seeing 
were really only to do with the EntryList bug(s), and that 
the changes to free nodes, and delete xmlnode's were 
not required, and may yet cause more headaches.

I definitely have to remove/reduce the amount of trace(4,
that has been added during this debugging since it's just
going to slow everything down, and makes to code rather
messy - look out for another commit in a few hours minus
all the tracing...