09.06.2016 15:23, Maxim Nestratov пишет:
09.06.2016 15:03, Peter Krempa пишет:
On Mon, Jun 06, 2016 at 20:59:22 +0300, 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.
Do you happen to have any kind of reproducer for this crash?
Unfortunately no.
Backtrace is the following:
Thread #1:
0 in pthread_cond_wait@@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@xxxxxxxxxxxxx>
---
src/util/virclosecallbacks.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/src/util/virclosecallbacks.c
b/src/util/virclosecallbacks.c
index 82d4071..2fab56b 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);
@@ -177,6 +178,7 @@ virCloseCallbacksUnset(virCloseCallbacksPtr
closeCallbacks,
goto cleanup;
virObjectUnref(vm);
+ virObjectUnref(closeCallbacks);
If this cleared the last reference, closeCallbacks is invalid at that
point.
Hm, yes, you are right. Actually I assumed that locking an object
guarantees the user of the object its validity. So I really thought
that locking function takes a reference and unlocking releases it. And
it turned out not to be true.
Effectively it's an error prone way of dealing with locking/unlocking
functions. Am I the only one who finds it a bit problematic? Or is
there some cases when you need to lock objects and not to reference
them that I don't see?
Disregard please. It's true only if you get a locked object from a list
or by some getter function, otherwise a caller should guarantee validity
of the object, which is untrue in my case and I'm trying to fix it
(eventually incorrectly).
I'll resend another version.
ret = 0;
cleanup:
virObjectUnlock(closeCallbacks);
But this would dereference and use it.
Peter
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list