On 05.09.2016 18:42, Maxim Nestratov wrote:
There is a possibility that qemu driver frees by unreferencing its
closeCallbacks pointer as it has the only reference to the object,
while in fact not all users of CloseCallbacks called thier
virCloseCallbacksUnset.
Backtrace is the following:
Thread #1:
0 in pthread_cond_wait@(a)GLIBC_2.3.2 () from /lib64/libpthread.so.0
1 in virCondWait (c=<optimized out>, m=<optimized out>)
at util/virthread.c:154
2 in virThreadPoolFree (pool=0x7f0810110b50)
at util/virthreadpool.c:266
3 in qemuStateCleanup () at qemu/qemu_driver.c:1116
4 in virStateCleanup () at libvirt.c:808
5 in main (argc=<optimized out>, argv=<optimized out>)
at libvirtd.c:1660
Thread #2:
0 in virClassIsDerivedFrom (klass=0xdeadbeef, parent=0x7f0837c694d0) at
util/virobject.c:169
1 in virObjectIsClass (anyobj=anyobj@entry=0x7f08101d4760, klass=<optimized out>)
at util/virobject.c:365
2 in virObjectLock (anyobj=0x7f08101d4760) at util/virobject.c:317
3 in virCloseCallbacksUnset (closeCallbacks=0x7f08101d4760, vm=vm@entry=0x7f08101d47b0,
cb=cb@entry=0x7f081d078fc0 <qemuProcessAutoDestroy>) at
util/virclosecallbacks.c:163
4 in qemuProcessAutoDestroyRemove (driver=driver@entry=0x7f081018be50,
vm=vm@entry=0x7f08101d47b0) at qemu/qemu_process.c:6368
5 in qemuProcessStop (driver=driver@entry=0x7f081018be50, vm=vm@entry=0x7f08101d47b0,
reason=reason@entry=VIR_DOMAIN_SHUTOFF_SHUTDOWN,
asyncJob=asyncJob@entry=QEMU_ASYNC_JOB_NONE, flags=flags@entry=0) at
qemu/qemu_process.c:5854
6 in processMonitorEOFEvent (vm=0x7f08101d47b0, driver=0x7f081018be50) at
qemu/qemu_driver.c:4585
7 qemuProcessEventHandler (data=<optimized out>, opaque=0x7f081018be50) at
qemu/qemu_driver.c:4629
8 in virThreadPoolWorker (opaque=opaque@entry=0x7f0837c4f820) at
util/virthreadpool.c:145
9 in virThreadHelper (data=<optimized out>) at util/virthread.c:206
10 in start_thread () from /lib64/libpthread.so.0
Let's reference CloseCallbacks object in virCloseCallbacksSet and
unreference in virCloseCallbacksUnset.
Signed-off-by: Maxim Nestratov <mnestratov(a)virtuozzo.com>
---
src/util/virclosecallbacks.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/src/util/virclosecallbacks.c b/src/util/virclosecallbacks.c
index 82d4071..891a92b 100644
--- a/src/util/virclosecallbacks.c
+++ b/src/util/virclosecallbacks.c
@@ -141,6 +141,7 @@ virCloseCallbacksSet(virCloseCallbacksPtr closeCallbacks,
virObjectRef(vm);
}
+ virObjectRef(closeCallbacks);
ret = 0;
cleanup:
virObjectUnlock(closeCallbacks);
@@ -180,6 +181,8 @@ virCloseCallbacksUnset(virCloseCallbacksPtr closeCallbacks,
ret = 0;
cleanup:
virObjectUnlock(closeCallbacks);
+ if (!ret)
+ virObjectUnref(closeCallbacks);
Might as well put this a few lines above, just before 'ret = 0' line.
ACK with that changed.
Michal