Re: [libvirt] [PATCH] rpc: avoid libvirtd crash on unexpected client close

On Mon, Aug 01, 2011 at 01:53:19PM -0600, Eric Blake wrote: > Steps to reproduce this problem (vm1 is not running): > for i in `seq 50`; do virsh managedsave vm1& done; killall virsh > > Pre-patch, virNetServerClientClose could end up setting client->sock > to NULL prior to other cleanup functions trying to use client->sock. > This fixes things by checking for NULL in more places, and by deferring > the cleanup until after all queued messages have been served. >

> @@ -570,10 +577,7 @@ void virNetServerClientClose(virNetServerClientPtr client) > virNetTLSSessionFree(client->tls); > client->tls = NULL; > } > - if (client->sock) { > - virNetSocketFree(client->sock); > - client->sock = NULL; > - } > + client->wantClose = true; > > while (client->rx) { > virNetMessagePtr msg > @@ -586,6 +590,11 @@ void virNetServerClientClose(virNetServerClientPtr client) > virNetMessageFree(msg); > } > > + if (client->sock) { > + virNetSocketFree(client->sock); > + client->sock = NULL; > + } > + > virNetServerClientUnlock(client); > } I'm curious why you have these last 2 hunks of the patc ? The entire of the virNetServerClientClose() is executed under the mutex, so AFAICT moving these 4 lines to later in the function can have no effect. The assignment to wantClose ought to not be needed at this point either, since this flag is used by the server to decide to invoke the virNetServerClientClose method.

That all said, these 2 hunks are at worst harmless, so ACK to the patch.