Hi On Sun, Oct 24, 2010 at 01:03:25PM -0400, Nick Mathewson wrote: > 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.
I'll add it to my list of things to try and do sometime but if someone out there has a Linux environment already set up for kernel hacking (or even knows who to report this to offhand) maybe they'll want to take a look first. > > 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. I think this makes sense and from what I can see in the epoll code it looks like it should work fine. Cheers > > 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.
