On 02/09/2017 08:12 PM, Wang King wrote:
If there is a process with a client which registers event callbacks,
and it calls libvirt's API which uses the same virConnectPtr in that
callback function. When this process exit abnormally lead to client
disconnect, there is a possibility that the main thread is refer to
virServerClient just after the virServerClient been freed by job
thread of libvirtd.
Could you give some more context on the code paths you're chasing...
Using libvirt function names and the sequence of events. A few details
on the setup path, then a bit more precision over the failure paths. I
assume this is a corner case, but it's really not clear. Is the issue
reproducible?
From just reading the commit message, I wonder if perhaps a
virObjectRef() needs to be done somewhere in order to ensure that
something that might be referencing something else doesn't get reaped
too soon, but the code below appears to be different...
Following is the backtrace:
#0 0x00007fda223d66d8 in virClassIsDerivedFrom
(klass=0xdeadbeef,parent=0x7fda24c81b40)
#1 0x00007fda223d6a1e in virObjectIsClass
(anyobj=anyobj@entry=0x7fd9e575b400,klass=<optimized out>)
#2 0x00007fda223d6a44 in virObjectLock (anyobj=anyobj@entry=0x7fd9e575b400)
#3 0x00007fda22507f71 in virNetServerClientSendMessage
(client=client@entry=0x7fd9e575b400, msg=msg@entry=0x7fd9ec30de90)
#4 0x00007fda230d714d in remoteDispatchObjectEventSend (client=0x7fd9e575b400,
program=0x7fda24c844e0, procnr=procnr@entry=348, proc=0x7fda2310e5e0
<xdr_remote_domain_event_callback_tunable_msg>, data=data@entry=0x7ffc3857fdb0)
#5 0x00007fda230dd71b in remoteRelayDomainEventTunable (conn=<optimized out>,
dom=0x7fda27cd7660, params=0x7fda27f3aae0, nparams=1, opaque=0x7fd9e6c99e00)
#6 0x00007fda224484cb in virDomainEventDispatchDefaultFunc (conn=0x7fda27cd0120,
event=0x7fda2736ea00, cb=0x7fda230dd610 <remoteRelayDomainEventTunable>,
cbopaque=0x7fd9e6c99e00)
#7 0x00007fda22446871 in virObjectEventStateDispatchCallbacks (callbacks=<optimized
out>, callbacks=<optimized out>, event=0x7fda2736ea00, state=0x7fda24ca3960)
#8 virObjectEventStateQueueDispatch (callbacks=0x7fda24c65800, queue=0x7ffc3857fe90,
state=0x7fda24ca3960)
#9 virObjectEventStateFlush (state=0x7fda24ca3960)
#10 virObjectEventTimer (timer=<optimized out>, opaque=0x7fda24ca3960)
#11 0x00007fda223ae8b9 in virEventPollDispatchTimeouts ()
#12 virEventPollRunOnce ()
#13 0x00007fda223ad1d2 in virEventRunDefaultImpl ()
#14 0x00007fda225046cd in virNetDaemonRun (dmn=dmn@entry=0x7fda24c775c0)
#15 0x00007fda230d6351 in main (argc=<optimized out>, argv=<optimized out>)
(gdb) p *(virNetServerClientPtr)0x7fd9e575b400
$2 = {parent = {parent = {u = {dummy_align1 = 140573849338048, dummy_align2 =
0x7fd9e65ac0c0, s = {magic = 3864707264, refs = 32729}}, klass = 0x7fda00000078}, lock =
{lock = {__data = {__lock = 0,
__count = 0, __owner = 0, __nusers = 0, __kind = 0, __spins = 0, __list =
{__prev = 0x0, __next = 0x0}}, __size = '\000' <repeats 39 times>, __align =
0}}}, wantClose = false,
delayedClose = false, sock = 0x0, auth = 0, readonly = false, tlsCtxt = 0x0, tls = 0x0,
sasl = 0x0, sockTimer = 0, identity = 0x0, nrequests = 0, nrequests_max = 0, rx = 0x0, tx
= 0x0, filters = 0x0,
nextFilterID = 0, dispatchFunc = 0x0, dispatchOpaque = 0x0, privateData = 0x0,
privateDataFreeFunc = 0x0, privateDataPreExecRestart = 0x0, privateDataCloseFunc = 0x0,
keepalive = 0x0}
---
src/rpc/virnetserverclient.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/src/rpc/virnetserverclient.c b/src/rpc/virnetserverclient.c
index 81da82c..562516f 100644
--- a/src/rpc/virnetserverclient.c
+++ b/src/rpc/virnetserverclient.c
@@ -1021,6 +1021,12 @@ void virNetServerClientClose(virNetServerClientPtr client)
client->sock = NULL;
}
+ if (client->privateData &&
+ client->privateDataFreeFunc) {
+ client->privateDataFreeFunc(client->privateData);
+ client->privateData = NULL;
+ }
+
I am wondering how this really fixes what you claim it does or if
perhaps virNetServerClientDispose needs to add that "client->privateData
= NULL;" when it gets run.
and this is very different from the commit description.
Tks -
John
virObjectUnlock(client);
}