[libvirt] [PATCH] rpc: Fix client crash on connection close

Jiri Denemark jdenemar at redhat.com
Mon Mar 5 10:56:25 UTC 2012


On Fri, Mar 02, 2012 at 16:12:16 -0700, Eric Blake wrote:
> On 03/02/2012 01:49 PM, Jiri Denemark wrote:
> > A multi-threaded client with event loop may crash if one of its threads
> > closes a connection while event loop is in the middle of sending
> > keep-alive message (either request or response). The right place for it
> > is inside virNetClientIOEventLoop() between poll() and
> > virNetClientLock(). We should only close a connection directly if no-one
> > is using it and defer the closing to the last user otherwise. So far we
> > only did so if the close was initiated by keep-alive timeout.
> > ---
> >  src/rpc/virnetclient.c |   18 ++++--------------
> >  1 files changed, 4 insertions(+), 14 deletions(-)
> > 
> 
> > @@ -512,19 +510,11 @@ virNetClientCloseLocked(virNetClientPtr client)
> >  
> >  void virNetClientClose(virNetClientPtr client)
> >  {
> > -    if (!client)
> > -        return;
> > -
> > -    virNetClientLock(client);
> > -    virNetClientCloseLocked(client);
> > -    virNetClientUnlock(client);
> > -}
> > -
> > -static void
> > -virNetClientRequestClose(virNetClientPtr client)
> > -{
> >      VIR_DEBUG("client=%p", client);
> 
> The diff that git picked is a bit confusing;

And I explicitly chose --patience diff since it generated better diff, which
represents exactly what I did :-)

> but it looks like all you are doing is stating that virNetClientClose should
> do the same thing as virNetClientRequestClose did (which is safer); and now
> that they do the same, you don't need two names, so pick the shorter name.

Right.

> ACK.

Thanks and pushed.

Jirka




More information about the libvir-list mailing list