From: Stefan Metzmacher Date: Wed, 13 Jul 2011 07:46:26 +0000 (+0200) Subject: tevent: add support for TEVENT_FD_ERROR X-Git-Tag: tevent-0.16.0~1 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=407cda2f3b7738d3690daeb8d679898f78ef3b74;p=thirdparty%2Fsamba.git tevent: add support for TEVENT_FD_ERROR After 12 years we finally got TEVENT_FD_ERROR support :-) TEVENT_FD_WRITE event handlers never get errors reported instead the event handler is silently disabled. There are likely callers relying on that behavior, so we are not able to chance it. Now TEVENT_FD_WRITE can be used together with TEVENT_FD_ERROR in order to get errors reported without waiting for TEVENT_FD_READ. TEVENT_FD_ERROR can also be used alone in order to detect errors on sockets in order to cleanup resources. Signed-off-by: Stefan Metzmacher Reviewed-by: Ralph Boehme --- diff --git a/lib/tevent/testsuite.c b/lib/tevent/testsuite.c index e33b475f324..e0881661756 100644 --- a/lib/tevent/testsuite.c +++ b/lib/tevent/testsuite.c @@ -1078,15 +1078,15 @@ static void test_event_fd3_writeable(struct tevent_context *ev_ctx, INT8_MAX, TEVENT_FD_WRITE, 9, - TEVENT_FD_READ|TEVENT_FD_WRITE, - TEVENT_FD_READ|TEVENT_FD_WRITE, - TEVENT_FD_READ|TEVENT_FD_WRITE, - TEVENT_FD_READ|TEVENT_FD_WRITE, - TEVENT_FD_READ|TEVENT_FD_WRITE, - TEVENT_FD_READ|TEVENT_FD_WRITE, - TEVENT_FD_READ|TEVENT_FD_WRITE, - TEVENT_FD_READ|TEVENT_FD_WRITE, - TEVENT_FD_READ|TEVENT_FD_WRITE); + TEVENT_FD_READ|TEVENT_FD_WRITE|TEVENT_FD_ERROR, + TEVENT_FD_READ|TEVENT_FD_WRITE|TEVENT_FD_ERROR, + TEVENT_FD_READ|TEVENT_FD_WRITE|TEVENT_FD_ERROR, + TEVENT_FD_READ|TEVENT_FD_WRITE|TEVENT_FD_ERROR, + TEVENT_FD_READ|TEVENT_FD_WRITE|TEVENT_FD_ERROR, + TEVENT_FD_READ|TEVENT_FD_WRITE|TEVENT_FD_ERROR, + TEVENT_FD_READ|TEVENT_FD_WRITE|TEVENT_FD_ERROR, + TEVENT_FD_READ|TEVENT_FD_WRITE|TEVENT_FD_ERROR, + TEVENT_FD_READ|TEVENT_FD_WRITE|TEVENT_FD_ERROR); } static void test_event_fd3_readable(struct tevent_context *ev_ctx, @@ -1110,15 +1110,15 @@ static void test_event_fd3_readable(struct tevent_context *ev_ctx, INT8_MAX, TEVENT_FD_READ|TEVENT_FD_WRITE, 9, - TEVENT_FD_READ|TEVENT_FD_WRITE, - TEVENT_FD_READ|TEVENT_FD_WRITE, - TEVENT_FD_READ|TEVENT_FD_WRITE, - TEVENT_FD_READ|TEVENT_FD_WRITE, - TEVENT_FD_READ|TEVENT_FD_WRITE, - TEVENT_FD_READ|TEVENT_FD_WRITE, - TEVENT_FD_READ|TEVENT_FD_WRITE, - TEVENT_FD_READ|TEVENT_FD_WRITE, - TEVENT_FD_READ|TEVENT_FD_WRITE); + TEVENT_FD_READ|TEVENT_FD_WRITE|TEVENT_FD_ERROR, + TEVENT_FD_READ|TEVENT_FD_WRITE|TEVENT_FD_ERROR, + TEVENT_FD_READ|TEVENT_FD_WRITE|TEVENT_FD_ERROR, + TEVENT_FD_READ|TEVENT_FD_WRITE|TEVENT_FD_ERROR, + TEVENT_FD_READ|TEVENT_FD_WRITE|TEVENT_FD_ERROR, + TEVENT_FD_READ|TEVENT_FD_WRITE|TEVENT_FD_ERROR, + TEVENT_FD_READ|TEVENT_FD_WRITE|TEVENT_FD_ERROR, + TEVENT_FD_READ|TEVENT_FD_WRITE|TEVENT_FD_ERROR, + TEVENT_FD_READ|TEVENT_FD_WRITE|TEVENT_FD_ERROR); test_event_fd3_prepare_phase(&state->sock1, __func__, @@ -1127,12 +1127,12 @@ static void test_event_fd3_readable(struct tevent_context *ev_ctx, 7, TEVENT_FD_READ, TEVENT_FD_READ|TEVENT_FD_WRITE, + TEVENT_FD_READ|TEVENT_FD_WRITE|TEVENT_FD_ERROR, 0, TEVENT_FD_READ, TEVENT_FD_WRITE, - 0, - TEVENT_FD_READ, - TEVENT_FD_WRITE, + TEVENT_FD_ERROR, + TEVENT_FD_WRITE|TEVENT_FD_ERROR, TEVENT_FD_READ|TEVENT_FD_WRITE); } @@ -1160,10 +1160,10 @@ static void test_event_fd3_not_writeable(struct tevent_context *ev_ctx, TEVENT_FD_WRITE, TEVENT_FD_READ, 0, - TEVENT_FD_READ|TEVENT_FD_WRITE, - TEVENT_FD_WRITE, - TEVENT_FD_READ, - 0, + TEVENT_FD_READ|TEVENT_FD_WRITE|TEVENT_FD_ERROR, + TEVENT_FD_WRITE|TEVENT_FD_ERROR, + TEVENT_FD_READ|TEVENT_FD_ERROR, + TEVENT_FD_ERROR, TEVENT_FD_READ); test_event_fd3_prepare_phase(&state->sock1, @@ -1171,15 +1171,15 @@ static void test_event_fd3_not_writeable(struct tevent_context *ev_ctx, INT8_MAX, TEVENT_FD_READ, 9, - TEVENT_FD_READ|TEVENT_FD_WRITE, - TEVENT_FD_READ|TEVENT_FD_WRITE, - TEVENT_FD_READ|TEVENT_FD_WRITE, - TEVENT_FD_READ|TEVENT_FD_WRITE, - TEVENT_FD_READ|TEVENT_FD_WRITE, - TEVENT_FD_READ|TEVENT_FD_WRITE, - TEVENT_FD_READ|TEVENT_FD_WRITE, - TEVENT_FD_READ|TEVENT_FD_WRITE, - TEVENT_FD_READ|TEVENT_FD_WRITE); + TEVENT_FD_READ|TEVENT_FD_WRITE|TEVENT_FD_ERROR, + TEVENT_FD_READ|TEVENT_FD_WRITE|TEVENT_FD_ERROR, + TEVENT_FD_READ|TEVENT_FD_WRITE|TEVENT_FD_ERROR, + TEVENT_FD_READ|TEVENT_FD_WRITE|TEVENT_FD_ERROR, + TEVENT_FD_READ|TEVENT_FD_WRITE|TEVENT_FD_ERROR, + TEVENT_FD_READ|TEVENT_FD_WRITE|TEVENT_FD_ERROR, + TEVENT_FD_READ|TEVENT_FD_WRITE|TEVENT_FD_ERROR, + TEVENT_FD_READ|TEVENT_FD_WRITE|TEVENT_FD_ERROR, + TEVENT_FD_READ|TEVENT_FD_WRITE|TEVENT_FD_ERROR); } static void test_event_fd3_off(struct tevent_context *ev_ctx, @@ -1200,16 +1200,16 @@ static void test_event_fd3_off(struct tevent_context *ev_ctx, test_event_fd3_prepare_phase(&state->sock1, __func__, INT8_MAX, - TEVENT_FD_READ|TEVENT_FD_WRITE, - 5, - TEVENT_FD_READ|TEVENT_FD_WRITE, - TEVENT_FD_WRITE, - TEVENT_FD_READ, - 0, + TEVENT_FD_READ|TEVENT_FD_WRITE|TEVENT_FD_ERROR, + 7, TEVENT_FD_READ|TEVENT_FD_WRITE, TEVENT_FD_WRITE, TEVENT_FD_READ, 0, + TEVENT_FD_READ|TEVENT_FD_WRITE|TEVENT_FD_ERROR, + TEVENT_FD_WRITE|TEVENT_FD_ERROR, + TEVENT_FD_READ|TEVENT_FD_ERROR, + TEVENT_FD_ERROR, TEVENT_FD_READ); } diff --git a/lib/tevent/tevent.h b/lib/tevent/tevent.h index b34b6ee394a..5aab01448f4 100644 --- a/lib/tevent/tevent.h +++ b/lib/tevent/tevent.h @@ -184,7 +184,7 @@ void tevent_set_default_backend(const char *backend); * * @param[in] fd The file descriptor to base the event on. * - * @param[in] flags #TEVENT_FD_READ or #TEVENT_FD_WRITE + * @param[in] flags #TEVENT_FD_READ, #TEVENT_FD_WRITE or #TEVENT_FD_ERROR. * * @param[in] handler The callback handler for the event. * @@ -535,8 +535,8 @@ void tevent_fd_set_auto_close(struct tevent_fd *fde); * * @param[in] fde File descriptor event to query * - * @return The flags set on the event. See #TEVENT_FD_READ and - * #TEVENT_FD_WRITE + * @return The flags set on the event. See #TEVENT_FD_READ, + * #TEVENT_FD_WRITE and #TEVENT_FD_ERROR */ uint16_t tevent_fd_get_flags(struct tevent_fd *fde); @@ -544,8 +544,8 @@ uint16_t tevent_fd_get_flags(struct tevent_fd *fde); * Set flags on a file descriptor event * * @param[in] fde File descriptor event to set - * @param[in] flags Flags to set on the event. See #TEVENT_FD_READ and - * #TEVENT_FD_WRITE + * @param[in] flags Flags to set on the event. See #TEVENT_FD_READ, + * #TEVENT_FD_WRITE and #TEVENT_FD_ERROR */ void tevent_fd_set_flags(struct tevent_fd *fde, uint16_t flags); @@ -563,13 +563,25 @@ void tevent_set_abort_fn(void (*abort_fn)(const char *reason)); /* bits for file descriptor event flags */ /** - * Monitor a file descriptor for data to be read + * Monitor a file descriptor for data to be read and errors + * + * Note: we map this from/to POLLIN, POLLHUP, POLLERR and + * where available POLLRDHUP */ #define TEVENT_FD_READ 1 /** * Monitor a file descriptor for writeability + * + * Note: we map this from/to POLLOUT */ #define TEVENT_FD_WRITE 2 +/** + * Monitor a file descriptor for errors + * + * Note: we map this from/to POLLHUP, POLLERR and + * where available POLLRDHUP + */ +#define TEVENT_FD_ERROR 4 /** * Convenience function for declaring a tevent_fd writable @@ -583,6 +595,12 @@ void tevent_set_abort_fn(void (*abort_fn)(const char *reason)); #define TEVENT_FD_READABLE(fde) \ tevent_fd_set_flags(fde, tevent_fd_get_flags(fde) | TEVENT_FD_READ) +/** + * Convenience function for declaring a tevent_fd waiting for errors + */ +#define TEVENT_FD_WANTERROR(fde) \ + tevent_fd_set_flags(fde, tevent_fd_get_flags(fde) | TEVENT_FD_ERROR) + /** * Convenience function for declaring a tevent_fd non-writable */ @@ -595,6 +613,12 @@ void tevent_set_abort_fn(void (*abort_fn)(const char *reason)); #define TEVENT_FD_NOT_READABLE(fde) \ tevent_fd_set_flags(fde, tevent_fd_get_flags(fde) & ~TEVENT_FD_READ) +/** + * Convenience function for declaring a tevent_fd not waiting for errors + */ +#define TEVENT_FD_NOT_WANTERROR(fde) \ + tevent_fd_set_flags(fde, tevent_fd_get_flags(fde) & ~TEVENT_FD_ERROR) + /** * Debug level of tevent */ diff --git a/lib/tevent/tevent_epoll.c b/lib/tevent/tevent_epoll.c index 43d5e37d1e2..e56bd026887 100644 --- a/lib/tevent/tevent_epoll.c +++ b/lib/tevent/tevent_epoll.c @@ -172,8 +172,27 @@ static void epoll_panic(struct epoll_event_context *epoll_ev, static uint32_t epoll_map_flags(uint16_t flags) { uint32_t ret = 0; - if (flags & TEVENT_FD_READ) ret |= (EPOLLIN | EPOLLERR | EPOLLHUP); - if (flags & TEVENT_FD_WRITE) ret |= (EPOLLOUT | EPOLLERR | EPOLLHUP); + + /* + * we do not need to specify EPOLLERR | EPOLLHUP + * they are always reported. + */ + + if (flags & TEVENT_FD_READ) { + /* + * Note that EPOLLRDHUP always + * returns EPOLLIN in addition, + * so EPOLLRDHUP is not strictly needed, + * but we want to make it explicit. + */ + ret |= EPOLLIN | EPOLLRDHUP; + } + if (flags & TEVENT_FD_WRITE) { + ret |= EPOLLOUT; + } + if (flags & TEVENT_FD_ERROR) { + ret |= EPOLLRDHUP; + } return ret; } @@ -538,10 +557,11 @@ static void epoll_update_event(struct epoll_event_context *epoll_ev, struct teve uint16_t effective_flags = tevent_common_fd_mpx_flags(primary); bool want_read = (effective_flags & TEVENT_FD_READ); bool want_write= (effective_flags & TEVENT_FD_WRITE); + bool want_error= (effective_flags & TEVENT_FD_ERROR); /* there's already an event */ if (primary->additional_flags & EPOLL_ADDITIONAL_FD_FLAG_HAS_EVENT) { - if (want_read || (want_write && !got_error)) { + if (want_read || want_error || (want_write && !got_error)) { epoll_mod_event(epoll_ev, primary); return; } @@ -556,7 +576,7 @@ static void epoll_update_event(struct epoll_event_context *epoll_ev, struct teve } /* there's no epoll_event attached to the fde */ - if (want_read || (want_write && !got_error)) { + if (want_read || want_error || (want_write && !got_error)) { epoll_add_event(epoll_ev, primary); return; } @@ -618,7 +638,7 @@ static int epoll_event_loop(struct epoll_event_context *epoll_ev, struct timeval return -1; } effective_flags = tevent_common_fd_mpx_flags(fde); - if (events[i].events & (EPOLLHUP|EPOLLERR)) { + if (events[i].events & (EPOLLHUP|EPOLLERR|EPOLLRDHUP)) { uint64_t add_flags = 0; add_flags |= EPOLL_ADDITIONAL_FD_FLAG_GOT_ERROR; @@ -626,6 +646,9 @@ static int epoll_event_loop(struct epoll_event_context *epoll_ev, struct timeval 0, add_flags); + if (effective_flags & TEVENT_FD_ERROR) { + flags |= TEVENT_FD_ERROR; + } if (effective_flags & TEVENT_FD_READ) { flags |= TEVENT_FD_READ; } diff --git a/lib/tevent/tevent_fd.c b/lib/tevent/tevent_fd.c index 6497d0d529c..a414f23f113 100644 --- a/lib/tevent/tevent_fd.c +++ b/lib/tevent/tevent_fd.c @@ -36,9 +36,10 @@ const char *tevent_common_fd_str(struct tevent_common_fd_buf *buf, { snprintf(buf->buf, sizeof(buf->buf), "%s[fde=%p," - "fd=%d,flags=0x%x(%s%s),%s]", + "fd=%d,flags=0x%x(%s%s%s),%s]", description, fde, fde->fd, fde->flags, + (fde->flags & TEVENT_FD_ERROR) ? "E" : "", (fde->flags & TEVENT_FD_READ) ? "R" : "", (fde->flags & TEVENT_FD_WRITE) ? "W" : "", fde->handler_name); diff --git a/lib/tevent/tevent_internal.h b/lib/tevent/tevent_internal.h index 70797d802e6..75ae114e35f 100644 --- a/lib/tevent/tevent_internal.h +++ b/lib/tevent/tevent_internal.h @@ -745,7 +745,9 @@ struct tevent_fd *tevent_common_fd_mpx_select(struct tevent_fd *primary, * If we got an error, we won't report it if * the caller only asked for TEVENT_FD_WRITE. */ - if (got_error && !(primary->flags & TEVENT_FD_READ)) { + if (got_error && + !(primary->flags & (TEVENT_FD_READ|TEVENT_FD_ERROR))) + { return NULL; } @@ -763,7 +765,9 @@ struct tevent_fd *tevent_common_fd_mpx_select(struct tevent_fd *primary, * If we got an error, we won't report it if * the caller only asked for TEVENT_FD_WRITE. */ - if (got_error && !(mpx_fde->flags & TEVENT_FD_READ)) { + if (got_error && + !(mpx_fde->flags & (TEVENT_FD_READ|TEVENT_FD_ERROR))) + { continue; } diff --git a/lib/tevent/tevent_poll.c b/lib/tevent/tevent_poll.c index 5c9c1bb56de..91a93817a8c 100644 --- a/lib/tevent/tevent_poll.c +++ b/lib/tevent/tevent_poll.c @@ -255,6 +255,43 @@ static struct tevent_fd *poll_event_add_fd(struct tevent_context *ev, return fde; } +/* + map from TEVENT_FD_* to POLLIN/POLLOUT +*/ +static uint16_t poll_map_flags(uint16_t flags) +{ + uint16_t pollflags = 0; + + /* + * we do not need to specify POLLERR | POLLHUP + * they are always reported. + */ + + if (flags & TEVENT_FD_READ) { + pollflags |= POLLIN; +#ifdef POLLRDHUP + /* + * Note that at least on Linux + * POLLRDHUP always returns + * POLLIN in addition, so this + * is not strictly needed, but + * we want to make it explicit. + */ + pollflags |= POLLRDHUP; +#endif + } + if (flags & TEVENT_FD_WRITE) { + pollflags |= POLLOUT; + } + if (flags & TEVENT_FD_ERROR) { +#ifdef POLLRDHUP + pollflags |= POLLRDHUP; +#endif + } + + return pollflags; +} + /* set the fd event flags */ @@ -263,7 +300,6 @@ static void poll_event_set_fd_flags(struct tevent_fd *fde, uint16_t flags) struct tevent_context *ev = fde->event_ctx; struct poll_event_context *poll_ev; uint64_t idx = fde->additional_flags; - uint16_t pollflags; if (ev == NULL) { return; @@ -308,15 +344,7 @@ static void poll_event_set_fd_flags(struct tevent_fd *fde, uint16_t flags) return; } - pollflags = 0; - - if (flags & TEVENT_FD_READ) { - pollflags |= (POLLIN|POLLHUP); - } - if (flags & TEVENT_FD_WRITE) { - pollflags |= (POLLOUT); - } - poll_ev->fds[idx].events = pollflags; + poll_ev->fds[idx].events = poll_map_flags(flags); poll_event_wake_pollthread(poll_ev); } @@ -402,16 +430,9 @@ static bool poll_event_sync_arrays(struct tevent_context *ev, } pfd->fd = fde->fd; - pfd->events = 0; + pfd->events = poll_map_flags(fde->flags); pfd->revents = 0; - if (fde->flags & TEVENT_FD_READ) { - pfd->events |= (POLLIN|POLLHUP); - } - if (fde->flags & TEVENT_FD_WRITE) { - pfd->events |= (POLLOUT); - } - poll_ev->num_fds += 1; } /* Both are in sync again */ @@ -539,17 +560,31 @@ static int poll_event_loop_poll(struct tevent_context *ev, continue; } - if (pfd->revents & (POLLHUP|POLLERR)) { - /* If we only wait for TEVENT_FD_WRITE, we - should not tell the event handler about it, - and remove the writable flag, as we only - report errors when waiting for read events - to match the select behavior. */ - if (!(fde->flags & TEVENT_FD_READ)) { +#ifdef POLLRDHUP +#define __POLL_RETURN_ERROR_FLAGS (POLLHUP|POLLERR|POLLRDHUP) +#else +#define __POLL_RETURN_ERROR_FLAGS (POLLHUP|POLLERR) +#endif + + if (pfd->revents & __POLL_RETURN_ERROR_FLAGS) { + /* + * If we only wait for TEVENT_FD_WRITE, we + * should not tell the event handler about it, + * and remove the writable flag, as we only + * report errors when waiting for read events + * or explicit for errors. + */ + if (!(fde->flags & (TEVENT_FD_READ|TEVENT_FD_ERROR))) + { TEVENT_FD_NOT_WRITEABLE(fde); continue; } - flags |= TEVENT_FD_READ; + if (fde->flags & TEVENT_FD_ERROR) { + flags |= TEVENT_FD_ERROR; + } + if (fde->flags & TEVENT_FD_READ) { + flags |= TEVENT_FD_READ; + } } if (pfd->revents & POLLIN) { flags |= TEVENT_FD_READ;