From fe8f3f19da88de963be2ef0d5c0908a9da0ffd2e Mon Sep 17 00:00:00 2001 From: Joachim Nilsson Date: Wed, 4 Mar 2015 09:41:22 +0100 Subject: [PATCH] Massively improve error handling, check for EPOLLHUP & EPOLLERR etc. - Make sure to listen for EPOLLRDHUP in watchers so users can be notified when a peer closes a connection. - Handle EPOLLHUP and EPOLLERR gracefully. Even handle spurious EPOLLHUP by means of an error counter. If we receive too many errors we now restart the epoll fd and re-add all watchers. - Return error from `epoll_ctl()` in `uev_watcher_stop()` - Ignore any error from `uev_watcher_stop()` in `uev_watcher_set()` - Move LIST_REMOVE() et al from io.c to core uev.c, should be done for timer and signal callbacks as well, of course. Signed-off-by: Joachim Nilsson --- io.c | 11 ++---- uev.c | 105 ++++++++++++++++++++++++++++++++++++++++++++++------------ uev.h | 1 + 3 files changed, 88 insertions(+), 29 deletions(-) diff --git a/io.c b/io.c index e0483c2..67e0a53 100644 --- a/io.c +++ b/io.c @@ -62,11 +62,8 @@ int uev_io_init(uev_ctx_t *ctx, uev_t *w, uev_cb_t *cb, void *arg, int fd, int e */ int uev_io_set(uev_t *w, int fd, int events) { - if (uev_io_stop(w)) - return -1; - - /* Remove from internal list */ - LIST_REMOVE(w, link); + /* Ignore any errors, only to clean up anything lingering ... */ + uev_io_stop(w); return uev_io_init(w->ctx, w, (uev_cb_t *)w->cb, w->arg, fd, events); } @@ -90,9 +87,7 @@ int uev_io_start(uev_t *w) */ int uev_io_stop(uev_t *w) { - int status = uev_watcher_stop(w); - - return status; + return uev_watcher_stop(w); } /** diff --git a/uev.c b/uev.c index 24696f4..db72c9f 100644 --- a/uev.c +++ b/uev.c @@ -24,6 +24,7 @@ */ #include +#include /* O_CLOEXEC */ #include /* memset() */ #include #include /* struct signalfd_siginfo */ @@ -33,6 +34,28 @@ #define UNUSED(arg) arg __attribute__ ((unused)) + +static int _init(uev_ctx_t *ctx, int close_old) +{ + int fd = epoll_create1(O_CLOEXEC); + + if (fd < 0) + return -1; + + if (close_old) + close(ctx->fd); + + ctx->fd = fd; + + return 0; +} + +/* Simple check if a descriptor is still valid in the kernel */ +static int is_valid_fd(int fd) +{ + return fcntl(fd, F_GETFL) != -1 || errno != EBADF; +} + /* Private to libuEv, do not use directly! */ int uev_watcher_init(uev_ctx_t *ctx, uev_t *w, uev_type_t type, uev_cb_t *cb, void *arg, int fd, int events) { @@ -41,16 +64,14 @@ int uev_watcher_init(uev_ctx_t *ctx, uev_t *w, uev_type_t type, uev_cb_t *cb, vo return -1; } - memset(w, 0, sizeof(*w)); w->ctx = ctx; - w->fd = fd; w->type = type; + w->active = 0; + w->fd = fd; w->cb = (void *)cb; w->arg = arg; w->events = events; - LIST_INSERT_HEAD(&w->ctx->watchers, w, link); - return 0; } @@ -67,13 +88,16 @@ int uev_watcher_start(uev_t *w) if (w->active) return 0; - ev.events = w->events; + ev.events = w->events | EPOLLRDHUP; ev.data.ptr = w; if (epoll_ctl(w->ctx->fd, EPOLL_CTL_ADD, w->fd, &ev) < 0) return -1; w->active = 1; + /* Add to internal list for bookkeeping */ + LIST_INSERT_HEAD(&w->ctx->watchers, w, link); + return 0; } @@ -88,10 +112,15 @@ int uev_watcher_stop(uev_t *w) if (!w->active) return 0; - /* Remove from kernel */ - epoll_ctl(w->ctx->fd, EPOLL_CTL_DEL, w->fd, NULL); w->active = 0; + /* Remove from internal list */ + LIST_REMOVE(w, link); + + /* Remove from kernel */ + if (epoll_ctl(w->ctx->fd, EPOLL_CTL_DEL, w->fd, NULL) < 0) + return -1; + return 0; } @@ -103,17 +132,15 @@ int uev_watcher_stop(uev_t *w) */ int uev_init(uev_ctx_t *ctx) { - int fd; - - fd = epoll_create(1); - if (fd < 0) + if (!ctx) { + errno = EINVAL; return -1; + } memset(ctx, 0, sizeof(*ctx)); - ctx->fd = fd; LIST_INIT(&ctx->watchers); - return 0; + return _init(ctx, 0); } /** @@ -206,20 +233,54 @@ int uev_run(uev_ctx_t *ctx, int flags) if (EINTR == errno) continue; /* Signalled, try again */ - exit: - result = -1; - ctx->running = 0; - break; + + /* Unrecoverable error, cleanup and exit with error. */ + uev_exit(ctx); + return -1; } for (i = 0; ctx->running && i < nfds; i++) { w = (uev_t *)events[i].data.ptr; + if (events[i].events & (EPOLLHUP | EPOLLERR)) { + ctx->errors++; + + if (ctx->errors >= 42) { + uev_t *tmp, *retry = w;; + + /* If not valid anymore, try to remove, ignore any errors. */ + if (!is_valid_fd(w->fd)) + uev_watcher_stop(w); + + /* Must recreate epoll fd now ... */ + if (_init(ctx, 1)) { + uev_exit(ctx); + return -2; + } + + /* Restart watchers in new efd */ + LIST_FOREACH_SAFE(w, &ctx->watchers, link, tmp) { + if (w->active) { + w->active = 0; + LIST_REMOVE(w, link); + uev_watcher_start(w); + } + } + uev_watcher_start(retry); + + /* New efd, restart everything! */ + ctx->errors = 0; + continue; + } + } + if (UEV_TIMER_TYPE == w->type) { uint64_t exp; - if (read(w->fd, &exp, sizeof(exp)) != sizeof(exp)) - goto exit; + if (read(w->fd, &exp, sizeof(exp)) != sizeof(exp)) { + uev_exit(ctx); + return -3; + } if (!w->period) w->timeout = 0; @@ -229,8 +290,10 @@ int uev_run(uev_ctx_t *ctx, int flags) struct signalfd_siginfo fdsi; ssize_t sz = sizeof(fdsi); - if (read(w->fd, &fdsi, sz) != sz) - goto exit; + if (read(w->fd, &fdsi, sz) != sz) { + uev_exit(ctx); + return -4; + } } if (w->cb) diff --git a/uev.h b/uev.h index f864e2d..f20fdb3 100644 --- a/uev.h +++ b/uev.h @@ -57,6 +57,7 @@ struct uev; typedef struct { int running; int fd; /* For epoll() */ + uint32_t errors; LIST_HEAD(,uev) watchers; } uev_ctx_t;