From 389a2fae802f0975846b2665afd1c76876387fd3 Mon Sep 17 00:00:00 2001 From: Bert Belder Date: Fri, 1 Dec 2017 22:51:15 +0100 Subject: [PATCH 1/5] test: use double underscores in '__declspec' --- test/shared/test-util.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/shared/test-util.h b/test/shared/test-util.h index bbf902c..314d54b 100644 --- a/test/shared/test-util.h +++ b/test/shared/test-util.h @@ -2,13 +2,13 @@ #define TEST_UTIL_H_ #ifdef _MSC_VER -#define no_return _declspec(noreturn) +#define no_return __declspec(noreturn) #else /* GCC/clang */ #define no_return __attribute__((noreturn)) #endif #ifdef _MSC_VER -#define no_inline _declspec(noinline) +#define no_inline __declspec(noinline) #else /* GCC/clang */ #define no_inline __attribute__((noinline)) #endif From 35b2354413e74a85e564fcb5a47d63fe3536d744 Mon Sep 17 00:00:00 2001 From: Bert Belder Date: Sun, 3 Dec 2017 18:14:28 +0100 Subject: [PATCH 2/5] doc: improve wording in epoll_close() documentation --- doc/README.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/doc/README.md b/doc/README.md index 26f7fba..a1665fa 100644 --- a/doc/README.md +++ b/doc/README.md @@ -73,8 +73,8 @@ int epoll_close(HANDLE ephnd); ``` * Close an epoll port. -* do not attempt to close the epoll port with Windows' `close`, - `CloseHandle` or `closesocket` functions. +* Do not attempt to close the epoll port with `close()`, + `CloseHandle()` or `closesocket()`. ### epoll_ctl From ddb8bfc9d68fdf76fc6516b0844637b24620adea Mon Sep 17 00:00:00 2001 From: Bert Belder Date: Sun, 3 Dec 2017 18:15:22 +0100 Subject: [PATCH 3/5] sock: properly detect closed sockets when EPOLLONESHOT is set --- src/sock.c | 31 +++++++++++++++++-------------- 1 file changed, 17 insertions(+), 14 deletions(-) diff --git a/src/sock.c b/src/sock.c index 56558d8..6e08978 100644 --- a/src/sock.c +++ b/src/sock.c @@ -36,12 +36,9 @@ typedef struct _ep_sock_private { } _ep_sock_private_t; static DWORD _epoll_events_to_afd_events(uint32_t epoll_events) { - DWORD afd_events; - - /* These events should always be monitored. */ - assert(epoll_events & EPOLLERR); - assert(epoll_events & EPOLLHUP); - afd_events = AFD_POLL_ABORT | AFD_POLL_CONNECT_FAIL | AFD_POLL_LOCAL_CLOSE; + /* Always monitor for AFD_POLL_LOCAL_CLOSE, which is triggered when the + * socket is closed with closesocket() or CloseHandle(). */ + DWORD afd_events = AFD_POLL_LOCAL_CLOSE; if (epoll_events & (EPOLLIN | EPOLLRDNORM)) afd_events |= AFD_POLL_RECEIVE | AFD_POLL_ACCEPT; @@ -51,6 +48,10 @@ static DWORD _epoll_events_to_afd_events(uint32_t epoll_events) { afd_events |= AFD_POLL_SEND | AFD_POLL_CONNECT; if (epoll_events & (EPOLLIN | EPOLLRDNORM | EPOLLRDHUP)) afd_events |= AFD_POLL_DISCONNECT; + if (epoll_events & EPOLLHUP) + afd_events |= AFD_POLL_ABORT; + if (epoll_events & EPOLLERR) + afd_events |= AFD_POLL_CONNECT_FAIL; return afd_events; } @@ -235,7 +236,9 @@ int ep_sock_set_event(ep_port_t* port_info, const struct epoll_event* ev) { _ep_sock_private_t* sock_private = _ep_sock_private(sock_info); - /* EPOLLERR and EPOLLHUP are always reported, even when no sollicited. */ + /* EPOLLERR and EPOLLHUP are always reported, even when not requested by the + * caller. However they are disabled after a event has been reported for a + * socket for which the EPOLLONESHOT flag as set. */ uint32_t events = ev->events | EPOLLERR | EPOLLHUP; sock_private->user_events = events; @@ -250,7 +253,7 @@ int ep_sock_set_event(ep_port_t* port_info, int ep_sock_update(ep_port_t* port_info, ep_sock_t* sock_info) { _ep_sock_private_t* sock_private = _ep_sock_private(sock_info); SOCKET driver_socket = poll_group_get_socket(sock_private->poll_group); - bool broken = false; + bool socket_closed = false; assert(ep_port_is_socket_update_pending(port_info, sock_info)); @@ -283,7 +286,7 @@ int ep_sock_update(ep_port_t* port_info, ep_sock_t* sock_info) { driver_socket) < 0) { if (GetLastError() == ERROR_INVALID_HANDLE) /* The socket is broken. It will be dropped from the epoll set. */ - broken = true; + socket_closed = true; else /* Another error occurred, which is propagated to the caller. */ return -1; @@ -301,7 +304,7 @@ int ep_sock_update(ep_port_t* port_info, ep_sock_t* sock_info) { ep_port_clear_socket_update(port_info, sock_info); /* If we saw an ERROR_INVALID_HANDLE error, drop the socket. */ - if (broken) + if (socket_closed) ep_sock_delete(port_info, sock_info); return 0; @@ -314,7 +317,7 @@ int ep_sock_feed_event(ep_port_t* port_info, container_of(overlapped, _ep_sock_private_t, poll_req.overlapped); ep_sock_t* sock_info = &sock_private->pub; uint32_t epoll_events; - bool drop_socket; + bool socket_closed; int ev_count = 0; sock_private->poll_status = _POLL_IDLE; @@ -327,7 +330,7 @@ int ep_sock_feed_event(ep_port_t* port_info, return 0; } - _poll_req_complete(&sock_private->poll_req, &epoll_events, &drop_socket); + _poll_req_complete(&sock_private->poll_req, &epoll_events, &socket_closed); /* Filter events that the user didn't ask for. */ epoll_events &= sock_private->user_events; @@ -344,10 +347,10 @@ int ep_sock_feed_event(ep_port_t* port_info, ev_count = 1; } - if (drop_socket) + if (socket_closed) /* Drop the socket from the epoll set. */ ep_sock_delete(port_info, sock_info); - else if (sock_private->user_events != 0) + else /* Put the socket back onto the attention list so a new poll request will * be submitted. */ ep_port_request_socket_update(port_info, sock_info); From af9180dcfb0dd03fea378578dad40a6d914ad1a9 Mon Sep 17 00:00:00 2001 From: Bert Belder Date: Sun, 3 Dec 2017 18:16:14 +0100 Subject: [PATCH 4/5] doc: document caveats related to auto-dropping of closed sockets --- doc/README.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/doc/README.md b/doc/README.md index a1665fa..2dac1f4 100644 --- a/doc/README.md +++ b/doc/README.md @@ -88,6 +88,11 @@ int epoll_ctl(HANDLE ephnd, * Control which socket events are monitored by an epoll port. * `ephnd` must be a HANDLE created by `epoll_create` or `epoll_create1`. * `op` must be one of `EPOLL_CTL_ADD`, `EPOLL_CTL_MOD`, `EPOLL_CTL_DEL`. +* It is recommended to always explicitly remove a socket from its epoll + set using `EPOLL_CTL_MOD` *before* closing it. As on Linux, sockets + are automatically removed from the epoll set when they are closed, but + wepoll may not be able to detect this until the next call to + `epoll_wait()`. * TODO: expand * [man page](http://man7.org/linux/man-pages/man2/epoll_ctl.2.html) From a0865a8677b7f64095fbed479b69825bf253adee Mon Sep 17 00:00:00 2001 From: Bert Belder Date: Sun, 3 Dec 2017 18:17:01 +0100 Subject: [PATCH 5/5] test: add test for automatic dropping or closed sockets --- test/test-auto-drop-on-close.c | 107 +++++++++++++++++++++++++++++++++ 1 file changed, 107 insertions(+) create mode 100644 test/test-auto-drop-on-close.c diff --git a/test/test-auto-drop-on-close.c b/test/test-auto-drop-on-close.c new file mode 100644 index 0000000..a43ce82 --- /dev/null +++ b/test/test-auto-drop-on-close.c @@ -0,0 +1,107 @@ +/* This test verifies that sockets are automatically dropped from an epoll set + * when they are closed. + * + * On Linux, a socket would be dropped from the epoll set immediately after the + * call to close(). However on Windows, this isn't possible - wepoll may not + * detect that a socket has been closed until a call to epoll_wait() is made. + * + * There are multiple code paths that detect a closed socket in the wepoll + * implementation; this test attempts to stress all of them. + */ + +#include +#include + +#include "test-util.h" +#include "util.h" +#include "wepoll.h" +#include "win.h" + +#include + +static SOCKET create_and_add_socket(HANDLE ephnd, int events) { + SOCKET sock; + struct epoll_event ev; + int r; + + /* Create UDP socket. */ + sock = socket(AF_INET, SOCK_DGRAM, IPPROTO_UDP); + check(sock != INVALID_SOCKET); + + /* Associate with epoll port. */ + ev.events = (uint32_t) events; + ev.data.sock = sock; + r = epoll_ctl(ephnd, EPOLL_CTL_ADD, sock, &ev); + check(r == 0); + + return sock; +} + +static void check_dropped(HANDLE ephnd, SOCKET sock) { + struct epoll_event ev; + int r; + + ev.events = EPOLLERR | EPOLLHUP; + ev.data.u64 = 0; + + /* Check that EPOLL_CTL_MOD fails. */ + r = epoll_ctl(ephnd, EPOLL_CTL_MOD, sock, &ev); + check(r < 0); + check(errno == ENOENT); /* TODO: should be EBADF. */ + + /* Check that EPOLL_CTL_DEL fails. */ + r = epoll_ctl(ephnd, EPOLL_CTL_DEL, sock, &ev); + check(r < 0); + check(errno == ENOENT); /* TODO: should be EBADF. */ +} + +int main(void) { + HANDLE ephnd; + SOCKET sock1, sock2, sock3, sock4, sock5; + struct epoll_event evs[8]; + int r; + + ephnd = epoll_create1(0); + check(ephnd != INVALID_HANDLE_VALUE); + + sock1 = create_and_add_socket(ephnd, EPOLLIN); + sock2 = create_and_add_socket(ephnd, EPOLLIN); + sock3 = create_and_add_socket(ephnd, EPOLLOUT); + sock4 = create_and_add_socket(ephnd, EPOLLOUT | EPOLLONESHOT); + sock5 = create_and_add_socket(ephnd, 0); + + r = closesocket(sock1); + check(r == 0); + + r = epoll_wait(ephnd, evs, array_count(evs), -1); + check(r == 2); /* sock3 and sock4 should report events. */ + + check_dropped(ephnd, sock1); + + r = closesocket(sock2); + check(r == 0); + r = closesocket(sock3); + check(r == 0); + + r = epoll_wait(ephnd, evs, array_count(evs), 0); + check(r == 0); /* No events should be reported. */ + + check_dropped(ephnd, sock2); + check_dropped(ephnd, sock3); + + r = closesocket(sock4); + check(r == 0); + r = closesocket(sock5); + check(r == 0); + + r = epoll_wait(ephnd, evs, array_count(evs), 0); + check(r == 0); /* No events should be reported. */ + + check_dropped(ephnd, sock4); + check_dropped(ephnd, sock5); + + r = epoll_close(ephnd); + check(r == 0); + + return 0; +}