Quick testing of the 0001 patch on my system seems ok. > -----Original Message----- > From: owner-libevent-us...@freehaven.net [mailto:owner-libevent- > us...@freehaven.net] On Behalf Of Nick Mathewson > Sent: Sunday, October 24, 2010 10:03 AM > To: libevent-us...@freehaven.net > Subject: Re: [Libevent-users] epoll erros > > On Fri, Oct 22, 2010 at 4:25 PM, Nicholas Marriott > <nicholas.marri...@gmail.com> 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 majord...@freehaven.net with unsubscribe libevent-users in the body.
