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 <metze@samba.org>
Reviewed-by: Ralph Boehme <slow@samba.org>

diff --git a/lib/tevent/testsuite.c b/lib/tevent/testsuite.c
index e33b475f3241e1140197df824f3cefd8e0090d62..e088166175647f9dcd0cec89377d0a175948c50c 100644 (file)
--- 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 b34b6ee394a2e55c62f1c0c3101013c40c581cd3..5aab01448f47800aace6e790bf07dc368552b41e 100644 (file)
--- 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 43d5e37d1e2cb5fc82c02c46e4314aaae5a9ce71..e56bd026887a5d2c6a9e886f9033893e13bdadc4 100644 (file)
--- 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 6497d0d529c32faf3648d3b6628eef8520146b06..a414f23f1133d656ba2ea0678a35d7819e3559c2 100644 (file)
--- 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 70797d802e6e5b3a1cea7a355436cb41e7c074fb..75ae114e35f8246499d9eedbfa9484b98157f6d1 100644 (file)
--- 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 5c9c1bb56de3c037f876edd0a11b60c6b5beb56d..91a93817a8c91866619dac0013a31ea935c53716 100644 (file)
--- 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;