On Mon, Apr 24, 2017 at 11:15:31AM +0200, Michal Privoznik wrote: > On 04/24/2017 05:31 AM, wang.yi59@xxxxxxxxxx wrote: > > Hi Michal, > > > > > > > > Thanks for your review. The problem occured in a python applicatin using libvirt-python, which has no ref(). If we unref() first in virKeepAliveTimer, we may get a segfault in virObjectUnlock() when cleanup, so I suppose that my patch is safer, :-) Here is the backtrace: > > > > > > Okay, now it makes more sense. However, the ref() I was referring to is our > libvirt virObjectRef() not the one in python. The code where segfault occurs > is too deep for python to see anyway. One thing that I am still worried > about is doing virObjectRef() before virObjectLock(). I think these two > steps should be swapped. That ordering should not matter. At the time this code runs, we *must* be guaranteed that we already hold a valid ref on the object. If we did not have that existing ref, then both virObjectRef and virObjectLock could segv. So the 'virObjectRef' must be acquiring a second reference, and thus doing lock first would not give us any further protection Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :| -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
