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