Re: xenstored file descriptor leak

[Manuel Bouyer <bouyer@xxxxxxxxxxxxxxx>]

On Wed, Feb 03, 2021 at 09:05:27AM +0100, Jürgen Groß wrote:
> > > [...]
> > > Yes, I think this is a good idea.
> > 
> > Well, after some sleep I don't think it is. We should always keep at last
> > POLLIN or we will never notice a socket close otherwise.
> 
> Adding the fd of an ignored socket connection to the list is the real
> problem here. Why should that be done?

If we don't do it, we never notice when the socket is closed and the file
descriptor will stay forever. When I tried it, I had about 50 zombie
file descriptors open in xenstored, after starting only 2 domains.

> > > > 
> > > > Now I wonder if, on NetBSD at last, a read error or short read shouldn't
> > > > cause the socket to be closed, as with:
> > > > 
> > > > @@ -1561,6 +1565,8 @@
> > > >    bad_client:
> > > >         ignore_connection(conn);
> > > > +       /* we don't want to keep this connection alive */
> > > > +       talloc_free(conn);
> > > >    }
> > > 
> > > This is wrong for non-socket connections, as we want to keep the domain
> > > in question to be known to xenstored.
> > > 
> > > For socket connections this should be okay, though.
> > 
> > What are "non-socket connections" BTW ? I don't think I've seen one
> > in my test.
> 
> Every connection to another domain.
> 
> > Is there a way to know if a connection is socket or non-socket ?
> 
> Active socket connections have conn->fd >= 0.

OK, I'll rework my patch. Thanks 

-- 
Manuel Bouyer <bouyer@xxxxxxxxxxxxxxx>
     NetBSD: 26 ans d'experience feront toujours la difference
--