Possible fix to 100% cpu usage with epoll and openssl

I'm running a fairly simple bit of test code using libevent2 with epoll and
openssl bufferevents and I've run into a 100% cpu usage problem.

Looking into it 100% usage was caused by epoll_wait constantly
returning write events on the openssl socket when it shouldn't really have
been looking for write events at all (N_ACTIVE_CALLBACKS() was returning 0
also).

Looking a bit deeper eventbuffer_openssl socket seems to be requesting
that the EV_WRITE event be removed when it should, but the event isn't
actually being removed from epoll.

Continuing to follow this I think I've found a bug in
event_changelist_del.

For evpoll event_del calls event_changelist_del which caches the change
which is then actioned later when evpoll_dispatch is called.

In event_changlist_del there is a check so that if the currently changed
action is an add then the cached action is changed to a no-op rather than a
delete (which makes sense). The problem arises if there are more than
two add or delete operations between calls to dispatch, in this case it's
possible that the delete is turned into a no-op when it shouldn't have
been.

For example starting with the event on, a delete followed by an add and
then another delete results in a no-op when it should have been a delete (I
added a fair bit of debug output that seems to confirm this behaviour).

I've applied a small change that checks the original old_event stored with
the change and only converts the delete to a no-op if the event isn't on in
old_event. This seems to have fixed my problem.

1 files changed, 12 insertions, 2 deletions

diff --git a/evmap.c b/evmap.c
index cb28002a..904eefdf 100644
--- a/evmap.c
+++ b/evmap.c
@@ -689,20 +689,30 @@ event_changelist_del(struct event_base *base, evutil_socket_t fd, short old, sho
 	   on those platforms where "add, delete, dispatch" is not the same
 	   as "no-op, dispatch", we want the no-op behavior.
 
+	   As well as checking the current operation we should also check
+	   the original set of events to make sure were not ignoring
+	   the case where the add operation is present on an event that 
+	   was already set.
+
 	   If we have a no-op item, we could remove it it from the list
 	   entirely, but really there's not much point: skipping the no-op
 	   change when we do the dispatch later is far cheaper than rejuggling
 	   the array now.
+
+	   As this stands, it also lets through deletions of events that are 
+	   not currently set.
 	 */
 
 	if (events & (EV_READ|EV_SIGNAL)) {
-		if (change->read_change & EV_CHANGE_ADD)
+                if (!(change->old_events & (EV_READ | EV_SIGNAL)) && 
+                    (change->read_change & EV_CHANGE_ADD))
 			change->read_change = 0;
 		else
 			change->read_change = EV_CHANGE_DEL;
 	}
 	if (events & EV_WRITE) {
-		if (change->write_change & EV_CHANGE_ADD)
+                if (!(change->old_events & EV_WRITE) && 
+                    (change->write_change & EV_CHANGE_ADD))
 			change->write_change = 0;
 		else
 			change->write_change = EV_CHANGE_DEL;