2009/9/29 Daniel P. Berrange <berrange@xxxxxxxxxx>: > On Sat, Sep 26, 2009 at 02:24:33AM +0200, Matthias Bolte wrote: >> I tracked down a memleak in libvirtd's message processing. The leak >> was introduced in commit 47cab734995fa9521b1df05d37e9978eedd8d3a2 >> "Split out code for handling incoming method call messages". This >> commit changed the way qemud_client_message objects were reused. >> >> Before this commit: >> >> - qemudWorker() removes a message from dispatch queue and passes it to >> remoteDispatchClientRequest() >> - remoteDispatchClientRequest() handles the message and reuses the >> same message for the response. If an error occurs the same message is >> used to report it (rpc_error label). If another error occurs while >> trying to report the first error remoteDispatchClientRequest() would >> return -1 and the message will be freed in qemudWorker() >> >> After this commit: >> >> - qemudWorker() removes a message from dispatch queue and passes it to >> remoteDispatchClientRequest() >> - remoteDispatchClientRequest() calls remoteDispatchClientCall() if >> the message it is an remote call or otherwise respond with a new error >> message by calling remoteSerializeReplyError() and the original >> message leaks >> - remoteDispatchClientCall() handles the message and reuses the same >> message for the response. If an error occurs it calls >> remoteSerializeReplyError() and the original message leaks. If a fatal >> error occurs remoteDispatchClientCall() returns -1 and the original >> message will be freed in qemudWorker() >> >> To fix this leak the original message has to be freed if >> remoteSerializeReplyError() succeeds. If remoteSerializeReplyError() >> fails the original message will be freed in qemudWorker(). >> >> In addition I came across another leak in remoteSerializeError(). If >> an error occurs while serializing the initial error the message leaks. >> To fix this leak the message has to be freed in case of an XDR error. > > ACK to your patch - this is all my fault caught up in the many times > I re-wrote the streams support. I really hate the contract of the > remoteDispatchClientRequest() method, it should always be responsible > for free'ing the 'msg' regardless of return status. I had actually > refactored things todo that, but then I rewrote it again and forget > that I had changed the error reporting functions thus leading to > this leak. > > Daniel Okay, I've committed the patch. Matthias -- Libvir-list mailing list Libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list