From ca4b640490de49dc3f8708429fbe0197377a2198 Mon Sep 17 00:00:00 2001 From: Azat Khuzhin Date: Wed, 31 Oct 2018 01:22:30 +0300 Subject: [PATCH] Merge branch 'event-ET-#636-v2' * event-ET-#636-v2: Preserve ET bit for backends with changelist Epoll ET setting lost with multiple events for same fd Cover ET with multiple events for same fd Add ET flag into event_base_dump_events() Fixes: #636 (cherry picked from commit 33053cdd8a9e1a7330b47759dee9cb209bed8f1b) --- epoll.c | 9 +++-- event.c | 3 +- evmap.c | 29 ++++++-------- test/regress_et.c | 100 ++++++++++++++++++++++++++++++++++++++++++---- 4 files changed, 114 insertions(+), 27 deletions(-) diff --git a/epoll.c b/epoll.c index bf730b23..a0df0d21 100644 --- a/epoll.c +++ b/epoll.c @@ -401,11 +401,14 @@ epoll_nochangelist_del(struct event_base *base, evutil_socket_t fd, ch.old_events = old; ch.read_change = ch.write_change = ch.close_change = 0; if (events & EV_WRITE) - ch.write_change = EV_CHANGE_DEL; + ch.write_change = EV_CHANGE_DEL | + (events & EV_ET); if (events & EV_READ) - ch.read_change = EV_CHANGE_DEL; + ch.read_change = EV_CHANGE_DEL | + (events & EV_ET); if (events & EV_CLOSED) - ch.close_change = EV_CHANGE_DEL; + ch.close_change = EV_CHANGE_DEL | + (events & EV_ET); return epoll_apply_one_change(base, base->evbase, &ch); } diff --git a/event.c b/event.c index caf766b5..ea72ddd3 100644 --- a/event.c +++ b/event.c @@ -3728,13 +3728,14 @@ dump_inserted_event_fn(const struct event_base *base, const struct event *e, voi if (! (e->ev_flags & (EVLIST_INSERTED|EVLIST_TIMEOUT))) return 0; - fprintf(output, " %p [%s "EV_SOCK_FMT"]%s%s%s%s%s%s", + fprintf(output, " %p [%s "EV_SOCK_FMT"]%s%s%s%s%s%s%s", (void*)e, gloss, EV_SOCK_ARG(e->ev_fd), (e->ev_events&EV_READ)?" Read":"", (e->ev_events&EV_WRITE)?" Write":"", (e->ev_events&EV_CLOSED)?" EOF":"", (e->ev_events&EV_SIGNAL)?" Signal":"", (e->ev_events&EV_PERSIST)?" Persist":"", + (e->ev_events&EV_ET)?" ET":"", (e->ev_flags&EVLIST_INTERNAL)?" Internal":""); if (e->ev_flags & EVLIST_TIMEOUT) { struct timeval tv; diff --git a/evmap.c b/evmap.c index 3f76dd0a..31ab4fee 100644 --- a/evmap.c +++ b/evmap.c @@ -393,7 +393,8 @@ evmap_io_del_(struct event_base *base, evutil_socket_t fd, struct event *ev) if (res) { void *extra = ((char*)ctx) + sizeof(struct evmap_io); - if (evsel->del(base, ev->ev_fd, old, res, extra) == -1) { + if (evsel->del(base, ev->ev_fd, + old, (ev->ev_events & EV_ET) | res, extra) == -1) { retval = -1; } else { retval = 1; @@ -858,6 +859,7 @@ event_changelist_add_(struct event_base *base, evutil_socket_t fd, short old, sh struct event_changelist *changelist = &base->changelist; struct event_changelist_fdinfo *fdinfo = p; struct event_change *change; + short evchange = EV_CHANGE_ADD | (events & (EV_ET|EV_PERSIST|EV_SIGNAL)); event_changelist_check(base); @@ -869,18 +871,12 @@ event_changelist_add_(struct event_base *base, evutil_socket_t fd, short old, sh * since the delete might fail (because the fd had been closed since * the last add, for instance. */ - if (events & (EV_READ|EV_SIGNAL)) { - change->read_change = EV_CHANGE_ADD | - (events & (EV_ET|EV_PERSIST|EV_SIGNAL)); - } - if (events & EV_WRITE) { - change->write_change = EV_CHANGE_ADD | - (events & (EV_ET|EV_PERSIST|EV_SIGNAL)); - } - if (events & EV_CLOSED) { - change->close_change = EV_CHANGE_ADD | - (events & (EV_ET|EV_PERSIST|EV_SIGNAL)); - } + if (events & (EV_READ|EV_SIGNAL)) + change->read_change = evchange; + if (events & EV_WRITE) + change->write_change = evchange; + if (events & EV_CLOSED) + change->close_change = evchange; event_changelist_check(base); return (0); @@ -893,6 +889,7 @@ event_changelist_del_(struct event_base *base, evutil_socket_t fd, short old, sh struct event_changelist *changelist = &base->changelist; struct event_changelist_fdinfo *fdinfo = p; struct event_change *change; + short del = EV_CHANGE_DEL | (events & EV_ET); event_changelist_check(base); change = event_changelist_get_or_construct(changelist, fd, old, fdinfo); @@ -919,19 +916,19 @@ event_changelist_del_(struct event_base *base, evutil_socket_t fd, short old, sh if (!(change->old_events & (EV_READ | EV_SIGNAL))) change->read_change = 0; else - change->read_change = EV_CHANGE_DEL; + change->read_change = del; } if (events & EV_WRITE) { if (!(change->old_events & EV_WRITE)) change->write_change = 0; else - change->write_change = EV_CHANGE_DEL; + change->write_change = del; } if (events & EV_CLOSED) { if (!(change->old_events & EV_CLOSED)) change->close_change = 0; else - change->close_change = EV_CHANGE_DEL; + change->close_change = del; } event_changelist_check(base); diff --git a/test/regress_et.c b/test/regress_et.c index f75c59b3..b9bd317e 100644 --- a/test/regress_et.c +++ b/test/regress_et.c @@ -51,6 +51,14 @@ static int was_et = 0; +static int base_supports_et(struct event_base *base) +{ + return + (!strcmp(event_base_get_method(base), "epoll") || + !strcmp(event_base_get_method(base), "epoll (with changelist)") || + !strcmp(event_base_get_method(base), "kqueue")); +} + static void read_cb(evutil_socket_t fd, short event, void *arg) { @@ -106,13 +114,7 @@ test_edgetriggered(void *et) /* Initalize the event library */ base = event_base_new(); - if (!strcmp(event_base_get_method(base), "epoll") || - !strcmp(event_base_get_method(base), "epoll (with changelist)") || - !strcmp(event_base_get_method(base), "kqueue")) - supports_et = 1; - else - supports_et = 0; - + supports_et = base_supports_et(base); TT_BLATHER(("Checking for edge-triggered events with %s, which should %s" "support edge-triggering", event_base_get_method(base), supports_et?"":"not ")); @@ -196,9 +198,93 @@ end: event_base_free(base); } +static int read_notification_count; +static int last_read_notification_was_et; +static void +read_notification_cb(evutil_socket_t fd, short event, void *arg) +{ + read_notification_count++; + last_read_notification_was_et = (event & EV_ET); +} + +static int write_notification_count; +static int last_write_notification_was_et; +static void +write_notification_cb(evutil_socket_t fd, short event, void *arg) +{ + write_notification_count++; + last_write_notification_was_et = (event & EV_ET); +} + +/* After two or more events have been registered for the same + * file descriptor using EV_ET, if one of the events is + * deleted, then the epoll_ctl() call issued by libevent drops + * the EPOLLET flag resulting in level triggered + * notifications. + */ +static void +test_edge_triggered_multiple_events(void *data_) +{ + struct basic_test_data *data = data_; + struct event *read_ev = NULL; + struct event *write_ev = NULL; + const char c = 'A'; + struct event_base *base = data->base; + int *pair = data->pair; + + if (!base_supports_et(base)) { + tt_skip(); + return; + } + + read_notification_count = 0; + last_read_notification_was_et = 0; + write_notification_count = 0; + last_write_notification_was_et = 0; + + /* Make pair[1] readable */ + tt_int_op(send(pair[0], &c, 1, 0), >, 0); + + read_ev = event_new(base, pair[1], EV_READ|EV_ET|EV_PERSIST, + read_notification_cb, NULL); + write_ev = event_new(base, pair[1], EV_WRITE|EV_ET|EV_PERSIST, + write_notification_cb, NULL); + + event_add(read_ev, NULL); + event_add(write_ev, NULL); + event_base_loop(base, EVLOOP_NONBLOCK|EVLOOP_ONCE); + event_base_loop(base, EVLOOP_NONBLOCK|EVLOOP_ONCE); + + tt_assert(last_read_notification_was_et); + tt_int_op(read_notification_count, ==, 1); + tt_assert(last_write_notification_was_et); + tt_int_op(write_notification_count, ==, 1); + + event_del(read_ev); + + /* trigger acitivity second time for the backend that can have multiple + * events for one fd (like kqueue) */ + close(pair[0]); + pair[0] = -1; + + /* Verify that we are still edge-triggered for write notifications */ + event_base_loop(base, EVLOOP_NONBLOCK|EVLOOP_ONCE); + event_base_loop(base, EVLOOP_NONBLOCK|EVLOOP_ONCE); + tt_assert(last_write_notification_was_et); + tt_int_op(write_notification_count, ==, 2); + +end: + if (read_ev) + event_free(read_ev); + if (write_ev) + event_free(write_ev); +} + struct testcase_t edgetriggered_testcases[] = { { "et", test_edgetriggered, TT_FORK, NULL, NULL }, { "et_mix_error", test_edgetriggered_mix_error, TT_FORK|TT_NEED_SOCKETPAIR|TT_NO_LOGS, &basic_setup, NULL }, + { "et_multiple_events", test_edge_triggered_multiple_events, + TT_FORK|TT_NEED_BASE|TT_NEED_SOCKETPAIR, &basic_setup, NULL }, END_OF_TESTCASES }; -- 2.40.0