Previously, we chose "ADD" whenever old_events==new_events, (since
we expected the add to fail with EEXIST), or whenever old_events
was==0, and MOD otherwise (i.e., when old_events was nonzero and not
equal to new_events).
But now that we retry failed MOD events as ADD *and* failed ADD
events as MOD, the important thing is now to try to guess right the
largest amount of the time, since guessing right means we do only
one syscall, but guessing wrong means we do two.
When old_events is 0, ADD is probably right (unless we're hitting
the dup bug, when we'll fall back).
And when old_events is set and != new_events, MOD is almost
certainly right for the same reasons as before.
But when old_events is equal to new events, then MOD will work fine
unless we closed and reopened the fd, in which case we'll have to
fall back to the ADD case. (Redundant del/add pairs are more common
than closes for most use cases.)
This change lets us avoid calculating new_events, which ought to
save a little time in epoll.c
Current versions of the Linux kernel don't seem to remove the struct
epitem for a given (file,fd) combo when the fd is closed unless the
file itself is also completely closed. This means that if you do:
fd = dup(fd_orig);
add(fd);
close(fd);
dup2(fd_orig, fd);
add(fd);
you will get an EEXIST when you should have gotten a success. This
could cause warnings and dropped events when using dup and epoll.
The solution is pretty simple: when we get an EEXIST from
EPOLL_CTL_ADD, we retry with EPOLL_CTL_MOD.
Unit test included to demonstrate the bug.
Found due to the patient efforts of Gilad Benjamini; diagnosed with
help from Nicholas Marriott.
We were trying to check whether any events had really been
notified on an fd before calling evmap_io_active on it, but instead
we were checking for an event pointer, which was always true.
In practice, this patch shouldn't change much, since epoll_wait
shouldn't return an event unless there is actually an event going
on.
Spotted by an anonymous bug reporter on Sourceforge. Closes bug
3078425.
It's okay for us to get an EPERM when doing an EPOLL_DEL on an fd; it
just means that before we got a chance to the EPOLL_DEL, we closed the
fd and reopened a new non-socket that wound up having the same fd.
Partial fix for Bug 3019973.
The logic here is a little complex, since epoll_add must used called exactly
when no events were previously set, epoll_mod must be used when any events
were previously set, and epoll_del only called when the removing all events.
Previously, our default lock model kind of assumed that every lock was
potentially a read-write lock. This was a poor choice, since
read-write locks are far more expensive than regular locks, and so the
lock API should only use them when we can actually take advantage of
them. Neither our pthreads or win32 lock implementation provided rw
locks.
Now that we have a way (not currently used!) to indicate that we
really want a read-write lock, we shouldn't actually say "lock this
for reading" or "lock this for writing" unless we mean it.
I've gone through everything that it declared to see where it was used,
and it seems that we probably don't need it anywhere.
Here's what it declared, and why I think we're okay dropping it.
o struct timeval {}
(Used all over, and we can't really get away with declaring it ourselves;
we need the same definition the system uses. If we can't find struct
timeval, we're pretty much sunk.)
o struct timespec {}
(Used in event.c, evdns.c, kqueue.c, evport.c. Of these,
kqueue.c and event.c include sys/_time.h. event.c conditions its use on
_EVENT_HAVE_CLOCK_GETTIME, and kqueue() only works if timespec is defined.)
o TIMEVAL_TO_TIMESPEC
(Used in kqueue.c, but every place with kqueue has sys/time.h)
o struct timezone {}
(event2/util.h has a forward declaration; only evutil.c references it and
doesn't look at its contents.)
o timerclear, timerisset, timercmp, timeradd, timersub
(Everything now uses the evutil_timer* variants.)
o ITIMER_REAL, ITIMER_VIRTUAL, ITIMER_PROF, struct itemerval
(These are only used in test/regress.c, which does not include _time.h)
o CLOCK_REALTIME
(Only used in evdns.c, which does not include _time.h)
o TIMESPEC_TO_TIMEVAL
o DST_*
o timespecclear, timespecisset, timespeccmp, timespecadd, timespecsub
o struct clockinfo {}
o CLOCK_VIRTUAL, CLOCK_PROF
o TIMER_RELTIME, TIMER_ABSTIME
(unused)
svn:r1494
This is harder than it sounds, since we need to make sure to
release the lock around the key call to the kernel (e.g.,
select, epoll_wait, kevent), AND we need to make sure that
none of the fields that are used in that call are touched by
anything that might be running concurrently in another
thread. I managed to do this pretty well for everything but
poll(). With poll, I needed to introduce a copy of the
event_set structure.
This patch also fixes a bug in win32.c where we called
realloc() instead of mm_realloc().
svn:r1450
We do this by not allocating the maximum epoll_event array for the epoll
backend at startup. Instead, we start out accepting 32 events at a time, and
double the array's size when it seems that the OS is generating events faster
than we're requesting them. This saves up to 374K per epoll-based
event_base. Resolves bug 2839240.
svn:r1428
a) this is 2009
b) niels and nick have been comaintainers for a while
c) saying "all rights reserved" when you then go on to explicitly
disclaim some rights is sheer cargo-cultism.
svn:r1065
Fix for epoll-on-linux bug (#1908866) where timeout values over (LONG_MAX-999)/HZ) (35 for me, or maybe 6 hours 50 min for some people, or maybe 3 hours 25 minutes for a special few) get treated as "wait forever". This actually deserves to be fixed in the kernel, but even if it is we will need to support Linux versions with this bug.
svn:r709
