Re: [libvirt] [PATCH 3/5] libvirtd: Alter refcnt processing for domain server objects

On 11/10/2017 10:08 AM, Erik Skultety wrote: I feel your pain ;-) Chasing these memory things is challenging especially when variable names change or things just aren't consistent. They're a real time sync too - that whole NetDaemon, NetServer, NetServerClient, etc. code is a beast to understand. I only have figured out a small piece of it... So let's see - I see the 2 REF's on @srv as you do in main(). The ctrl->daemon->servers will be UNREF'd in virNetDaemonDispose when the last Unref of ctrl->daemon is done in virLXCControllerFree via the call to virHashFree... BTW: Chasing that is painful, but once the server is removed from the hash table it does end up calling an Unref when virObjectFreeHashData is called (e.g. the 2nd arg to virHashCreate). This may be one of those cases where you need to add "temporary" debug code to virObjectRef and virObjectUnref along with the address of the object in order to see "when" it's Ref'd and Unref'd and what the count is (I've done this using VIR_WARN and just perusing generated output). It is painful... Don't forget to add debug code to this function that would print the addr of the daemon and server object's so that you can output everything to a file and then comb through the output looking for when things are ref/unref'd and most importantly some free of an object that didn't have an object it contains being free'd. Once you see the daemon object free'd, but the @srv object still hanging on - you know you're pretty safe in assuming there's a missing Unref... Anyway, I agree with you upon visual inspection - there's a leak since @srv is never Unref'd. Once it's added to the Server hash table, then there should be a virObjectUnref(srv). As for other consumers.... The reason that log_daemon doesn't do the virObjectUnref is that it saves @srv and eventually will Unref it later. Unlike the lock_daemon code which uses virNetDaemonGetServer to find the @srv; whereas, log_daemon goes direct to logDaemon->srv. The REF is for the Hash Table. We've added something to the hash table - we should increment the refcnt - nothing more nothing less. The UNREF for that occurs as part of virHashFree because virHashCreate uses virObjectFreeHashData. The UNREF is for the allocation (or virNetServerNew) and an optimization because we know it's in the Hash Table and wouldn't be removed from the Hash Table until the virNetDaemonDispose calls virHashFree once the last @dmn is UNREF'd. Dropping this patch and the next one is fine by me, but the Unref's below would be reinstated. That messes up log_daemon as it's stores @srv in logDamon->srv and then will virObjectUnref it virLogDaemonFree. If log_daemon didn't save a ref of the object in logDaemon->srv, then it too could do a virObjectUnref right away too.