RE: [Libevent-users] epoll erros

["Gilad Benjamini" <gilad@xxxxxxxxxxxxxxxxx>]

Quick testing of the 0001 patch on my system seems ok.

> -----Original Message-----
> From: owner-libevent-users@xxxxxxxxxxxxx [mailto:owner-libevent-
> users@xxxxxxxxxxxxx] On Behalf Of Nick Mathewson
> Sent: Sunday, October 24, 2010 10:03 AM
> To: libevent-users@xxxxxxxxxxxxx
> Subject: Re: [Libevent-users] epoll erros
> 
> On Fri, Oct 22, 2010 at 4:25 PM, Nicholas Marriott
> <nicholas.marriott@xxxxxxxxx> wrote:
> > I think something needs to walk the file->f_ep_links list on close()
> > and remove any epitems where epi->ffd->fd is the fd being closed from
> > the tree in epi->ep.
> >
> > I don't have a Linux box to try this on though :-).
> 
> I think you're right.  I don't do kernel hacking myself though, so
> maybe somebody else should take this one on.
> 
> In the meantime, this doesn't help people using any of the kernels
> currently in the wild.  Fortunately, part of the point of Libevent is
> to work around weird kernel behaviors.  Since the epoll tree _does_
> keep a separate struct epitem for every (file, fd) pair, I believe that
> we can work around this behavior by having epoll fall back to
> EPOLL_CTL_MOD when EPOLL_CTL_ADD fails with EEXIST.
> 
> Incidentally, there's another, more sinister bug hiding here.  When
> Libevent's changelist code coalesces a (DEL,ADD) combo, rather than
> making it a no-op, it makes it into an "ADD", since it's possible that
> the fd was closed in between the DEL and the ADD.  It calls these adds
> "precautionary".  When the current epoll code encounters such an add
> and the add fails with EEXIST, we currently ignore the failure with a
> debug message.  But if the user has been using dup() in the same way as
> Gilad's original code, this means that they'll think they have added
> the event, when in fact they haven't.
> 
> I've attached a series of two patches that try to get working behavior
> here.  See the commit messages for details.  Please test them if you
> can; they'll probably go into 2.0.9 if nobody finds them to be buggy.
> They are also branch epoll_dup in my github repo.
> 
> In the process, I also decided that maybe epoll_apply_changes should
> have fewer conditional branches, since they tend to be a poor choice on
> modern CPUs.  I've got a branch epoll_table in my github repo that
> replaces the huge pile of conditionals at the start of
> eopll_apply_changes with a table lookup.  It is *NOT* under
> consideration for 2.0, since it's a relatively large change and it
> doesn't fix a bug, but if anybody wants to have a look, that would be
> cool tool.
> 
> (My github repo is at git://github.com/nmathewson/Libevent.git )
> 
> yrs,
> --
> Nick

***********************************************************************
To unsubscribe, send an e-mail to majordomo@xxxxxxxxxxxxx with
unsubscribe libevent-users    in the body.