On 07/13/2017 06:54 PM, John Ferlan wrote: > > > On 07/11/2017 11:52 AM, Michal Privoznik wrote: >> On 06/03/2017 03:27 PM, John Ferlan wrote: >>> Since the virSecretObjListAdd technically consumes @def on success, >>> the secretDefineXML should fetch the @def from the object afterwards >>> and manage as an @objdef and set @def = NULL immediately. >> >> Really? virSecretObjListAdd sets ->def pointer in the object, but it >> doesn't touch the definition otherwise. So I think a caller is safe to >> continue using the pointer. >> >> Michal >> > > Let's consider the code before my change... > > @def is added to the @obj > > "Something" causes us to jump to the "restore_backup:" label and @backup > == NULL. > > That means virSecretObjListRemove runs and because @obj has @def it ends > up calling virSecretDefFree The only way this can happen is when @obj has refcnt == 1. Because then unref() calls dispose() which calls virSecretDefFree(). However, @obj will have at least 2 references when entering restore_backup label. In virSecretObjListAdd() when creating the new object via virSecretObjNew() obj has refcnt = 1, and then we ref the object again. But wait a second: if the object is already in both of the hash tables we return that reference and don't increase the refcnt! So in the end, virSecretObjListAdd() can return an object with refcnt == 1 and refcnt == 2. This is obviously wrong and root cause of the problem you are seeing. As I describe in the other e-mail, this breaks refcounting and needs to be fixed. Michal -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
