Re: [Libvir] PATCH: Fix and cleanup ref counting/ domain/network object release

On Sat, Jan 19, 2008 at 07:18:03PM +0000, Daniel P. Berrange wrote: > The referencing counting code for Connect/Domain/Network objects has many > repeated codepaths, not all of which are correct. eg, the virFreeDomain > method forgets to release networks when garbage collecting a virConnectPtr, > and the virFreeNetwork method forgets to release domains. I've also found > it very confusing having both virFreeDomain and virDomainFree. > > So I've changed the impl of the cleanup functions in hash.c to use a > slightly different structure. There are now 6 functions: > > - virUnrefConnect > - virReleaseConnect > - virUnrefDomain > - virReleaseDomain > - virUnrefNetwork > > The 'virUnref*' APIs are intended for use by the drivers to decrement the > ref count on objects they no loner need. In the virUnref* method, if the > ref count hits zero, then it calls the corresponding virRelease* method > to actually free the object's memory. The virUnref* methods are responsible > for acquiring the connection mutex. The virRelease* method mandate that5D > the mutex is already held, and will releae it as the very last step when > deleting the object. The virReleaseDomain/virReleaeNetwork methods will > also derecement the ref count of the connection, and call virReleaseConnection > if appropriate. The patch for all this looks horrific but its not as bad as > it looks, so I'd recommend reviewing the final code in hash.c, rather than > the patch. Sounds okay, > I have also switched from xmlMutex to the POSIX standard pthread_mutex_t > object. In theory the former may give more portability, but the patches which > follow are also going to require actual pthread_t objects, for which libxml > has no wrappers. Thus it is pointless using the xmlMutex abstraction. The > added plus, is that pthread_mutex_t objects do not require any memory > allocation, merely initialization of their pre-allocated contents so it > simplifies a little code. That I found worrying. What is the added benefit for the loss of portability? You now reference 2 relatively system specific kind of data structure, where we used to reference only the xmlMutex one which was portable. And i don't see why, you need to do this for the goal stated before.