[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH v2 2/2] remote: free client all event callbacks at remoteClientCloseFunc



On Mon, Nov 13, 2017 at 09:07:12PM +0800, xinhua.Cao wrote:


在 2017/11/12 22:45, John Ferlan 写道:

On 11/11/2017 03:20 AM, xinhua.Cao wrote:

在 2017/11/11 5:43, John Ferlan 写道:
On 11/06/2017 12:47 AM, xinhua.Cao wrote:
base on commit fe8f1c8b8650c8ab5e7d27338f4538582651bd14, we solve
libvirt coredump problem, but it introduce a memory leak sense:

1. one client register a domain event such as reboot event
2. and client was terminated unexpectly, such as kill -9,
unexpectedly

then this client object will not free at libvirtd service program.

remoteDispatchConnectDomainEventCallbackRegisterAny reference the
client,
but when client was terminated before it call deRegisterAny,the
reference
of client will not reduced to zero. so the memory leak take place.
What's the difference between the path that would terminate normally and
the one that terminates abnormally. It seems to me there might be (an)
extra Ref(s) on the client which prevents virNetServerClientDispose from
calling it's @privateDataFreeFunc which would call remoteClientFreeFunc
since remoteClientInitHook worked properly.

That's what needs to be found.
If client terminate normally, client would call
virConnectDomainEventDeregisterAny to
clear eventCallbacks. Then libvirt daemon will call
remoteDispatchConnectDomainEventDeregisterAny
-> remoteEventCallbackFree -> virObjectUnref(callback->client) to free
eventCallbacks and unRef client.
just think if client don't call  virConnectDomainEventDeregisterAny and
be killed. then there is no way to call call
remoteDispatchConnectDomainEventDeregisterAny -> remoteEventCallbackFree
-> virObjectUnref(callback->client)
so client's ref can't down to zero. so remoteClientFreeFunc can't be
called. then client object memory leak.
So the client/sample code is not calling Deregister because it has no
"handling" of an abnormal exit. That is no signal handler to call
Deregister in the event some outside force kills the client/sample code
while it's processing.

So a different solution would be to add signal handling to the code in
order to handle the kill and make the Deregister (and close) calls.

signal handler can handle such as kill -15 signal, But can't handle kill
-9 signal.
so we also need to resolve it in libvirt

Basically we need to handle bad behaving clients in the daemon anyway.  I tried
running libvirtd under valgrind and there was no extra leak with a client which
didn't deregister the callback.  How did you encounter this?  I really want to
find the place that I think needs to be changed, but without a reproducer this
is just a guessing game.

Attachment: signature.asc
Description: Digital signature


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]