On 03/27/2017 06:47 PM, John Ferlan wrote:
> Based upon an idea and some research by Wang King <king.wang@xxxxxxxxxx>
> and xinhua.Cao <caoxinhua@xxxxxxxxxx>.
>
> Since we're assigning the 'client' to our callback event lookaside list,
> it's imperative that we grab a reference to the object; otherwise, when
> the object is unref'd during virNetServerProcessClients when it's determined
> that the virNetServerClientIsClosed and the memory is free'd before perhaps
> the object event state callbacks are run. When a virObjectLock() is run,
> before sending the message the following trace occurs;
>
> #0 0x00007fda223d66d8 in virClassIsDerivedFrom
> (klass=0xdeadbeef, parent=0x7fda24c81b40)
> at util/virobject.c:169
> #1 0x00007fda223d6a1e in virObjectIsClass
> (anyobj=anyobj@entry=0x7fd9e575b400, klass=<optimized out>)
> at util/virobject.c:365
> #2 0x00007fda223d6a44 in virObjectLock
> (anyobj=0x7fd9e575b400)
> at util/virobject.c:317
> #3 0x00007fda22507f71 in virNetServerClientSendMessage
> (client=client@entry=0x7fd9e575b400, msg=msg@entry=0x7fd9ec30de90)
> at rpc/virnetserverclient.c:1422
> #4 0x00007fda230d714d in remoteDispatchObjectEventSend
> (client=0x7fd9e575b400, program=0x7fda24c844e0, procnr=348,
> proc=0x7fda2310e5e0 <xdr_remote_domain_event_callback_tunable_msg>,
> data=0x7ffc3857fdb0)
> at remote.c:3803
> #5 0x00007fda230dd71b in remoteRelayDomainEventTunable
> (conn=<optimized out>, dom=0x7fda27cd7660, params=0x7fda27f3aae0,
> nparams=1,opaque=0x7fd9e6c99e00)
> at remote.c:1033
> #6 0x00007fda224484cb in virDomainEventDispatchDefaultFunc
> (conn=0x7fda27cd0120, event=0x7fda2736ea00, cb=0x7fda230dd610
> <remoteRelayDomainEventTunable>, cbopaque=0x7fd9e6c99e00)
> at conf/domain_event.c:1910
> #7 0x00007fda22446871 in virObjectEventStateDispatchCallbacks
> (callbacks=<optimized out>, callbacks=<optimized out>,
> event=0x7fda2736ea00,state=0x7fda24ca3960)
> at conf/object_event.c:722
> #8 virObjectEventStateQueueDispatch
> (callbacks=0x7fda24c65800, queue=0x7ffc3857fe90, state=0x7fda24ca3960)
> at conf/object_event.c:736
> #9 virObjectEventStateFlush (state=0x7fda24ca3960)
> at conf/object_event.c:814
> #10 virObjectEventTimer (timer=<optimized out>, opaque=0x7fda24ca3960)
> at conf/object_event.c:560
> #11 0x00007fda223ae8b9 in virEventPollDispatchTimeouts ()
> at util/vireventpoll.c:458
> #12 virEventPollRunOnce ()
> at util/vireventpoll.c:654
> #13 0x00007fda223ad1d2 in virEventRunDefaultImpl ()
> at util/virevent.c:314
> #14 0x00007fda225046cd in virNetDaemonRun (dmn=0x7fda24c775c0)
> at rpc/virnetdaemon.c:818
> #15 0x00007fda230d6351 in main (argc=<optimized out>, argv=<optimized out>)
> at libvirtd.c:1623
>
> Signed-off-by: John Ferlan <jferlan@xxxxxxxxxx>
> ---
> daemon/remote.c | 38 +++++++++++++++++++++-----------------
> 1 file changed, 21 insertions(+), 17 deletions(-)
>
> diff --git a/daemon/remote.c b/daemon/remote.c
> index 5cdc53e..25a29cf 100644
> --- a/daemon/remote.c
> +++ b/daemon/remote.c
> @@ -124,7 +124,11 @@ remoteDispatchObjectEventSend(virNetServerClientPtr client,
> static void
> remoteEventCallbackFree(void *opaque)
> {
> - VIR_FREE(opaque);
> + daemonClientEventCallbackPtr callback = opaque;
> + if (!callback)
> + return;
> + virObjectUnref(callback->client);
> + VIR_FREE(callback);
> }
>
>
> @@ -3896,7 +3900,7 @@ remoteDispatchConnectDomainEventRegister(virNetServerPtr server ATTRIBUTE_UNUSED
> */
> if (VIR_ALLOC(callback) < 0)
> goto cleanup;
> - callback->client = client;
> + callback->client = virObjectRef(client);
> callback->eventID = VIR_DOMAIN_EVENT_ID_LIFECYCLE;
> callback->callbackID = -1;
> callback->legacy = true;
> @@ -3923,7 +3927,7 @@ remoteDispatchConnectDomainEventRegister(virNetServerPtr server ATTRIBUTE_UNUSED
> rv = 0;
>
> cleanup:
> - VIR_FREE(callback);
> + remoteEventCallbackFree(callback);
> if (rv < 0)
> virNetMessageSaveError(rerr);
> virMutexUnlock(&priv->lock);
I don't really see how this could work in the first place. I mean,
@callback is allocated in the chunk above. Cool. Then, in between these
two chunks it's passed (under code name @ref) to
virConnectDomainEventRegisterAny() ...
Aaand I get it now. Just realized that VIR_APPEND_ELEMENT() sets
@callback to NULL. So we are safe here. D'oh.
ACK
Michal
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
