From 0f25aeccebda94a7708ebbf7beadf2bb606d1d66 Mon Sep 17 00:00:00 2001 From: Mihai Renea Date: Fri, 31 Jan 2025 14:27:47 +0100 Subject: [PATCH] sock/async: cancel sock async events when closing the socket --- pkg/lwip/contrib/sock/ip/lwip_sock_ip.c | 7 ++ pkg/lwip/contrib/sock/tcp/lwip_sock_tcp.c | 8 ++ pkg/lwip/contrib/sock/udp/lwip_sock_udp.c | 7 ++ pkg/openwsn/sock/openwsn_sock_udp.c | 3 + pkg/tinydtls/contrib/sock_dtls.c | 3 + sys/include/net/sock/async/event.h | 54 +++++++++ sys/net/gnrc/sock/ip/gnrc_sock_ip.c | 7 ++ sys/net/gnrc/sock/udp/gnrc_sock_udp.c | 7 ++ sys/net/sock/async/event/sock_async_event.c | 115 ++++++++++++++++++++ 9 files changed, 211 insertions(+) diff --git a/pkg/lwip/contrib/sock/ip/lwip_sock_ip.c b/pkg/lwip/contrib/sock/ip/lwip_sock_ip.c index 74f9383853..5165690803 100644 --- a/pkg/lwip/contrib/sock/ip/lwip_sock_ip.c +++ b/pkg/lwip/contrib/sock/ip/lwip_sock_ip.c @@ -30,6 +30,10 @@ #include "lwip/sys.h" #include "lwip/sock_internal.h" +#ifdef SOCK_HAS_ASYNC_CTX +#include "net/sock/async/event.h" +#endif + int sock_ip_create(sock_ip_t *sock, const sock_ip_ep_t *local, const sock_ip_ep_t *remote, uint8_t proto, uint16_t flags) { @@ -60,6 +64,9 @@ void sock_ip_close(sock_ip_t *sock) netconn_delete(sock->base.conn); sock->base.conn = NULL; } +#ifdef SOCK_HAS_ASYNC_CTX + sock_event_close(sock_ip_get_async_ctx(sock)); +#endif } int sock_ip_get_local(sock_ip_t *sock, sock_ip_ep_t *ep) diff --git a/pkg/lwip/contrib/sock/tcp/lwip_sock_tcp.c b/pkg/lwip/contrib/sock/tcp/lwip_sock_tcp.c index 0865ec183a..0fa9596a6e 100644 --- a/pkg/lwip/contrib/sock/tcp/lwip_sock_tcp.c +++ b/pkg/lwip/contrib/sock/tcp/lwip_sock_tcp.c @@ -24,6 +24,10 @@ #include "lwip/api.h" #include "lwip/opt.h" +#ifdef SOCK_HAS_ASYNC_CTX +#include "net/sock/async/event.h" +#endif + static inline void _tcp_sock_init(sock_tcp_t *sock, struct netconn *conn, sock_tcp_queue_t *queue) { @@ -125,6 +129,10 @@ void sock_tcp_disconnect(sock_tcp_t *sock) } } +#ifdef SOCK_HAS_ASYNC_CTX + sock_event_close(sock_tcp_get_async_ctx(sock)); +#endif + mutex_unlock(&sock->mutex); memset(&sock->mutex, 0, sizeof(mutex_t)); } diff --git a/pkg/lwip/contrib/sock/udp/lwip_sock_udp.c b/pkg/lwip/contrib/sock/udp/lwip_sock_udp.c index cb5e22a587..d119d153ec 100644 --- a/pkg/lwip/contrib/sock/udp/lwip_sock_udp.c +++ b/pkg/lwip/contrib/sock/udp/lwip_sock_udp.c @@ -27,6 +27,10 @@ #include "lwip/sys.h" #include "lwip/udp.h" +#ifdef SOCK_HAS_ASYNC_CTX +#include "net/sock/async/event.h" +#endif + int sock_udp_create(sock_udp_t *sock, const sock_udp_ep_t *local, const sock_udp_ep_t *remote, uint16_t flags) { @@ -56,6 +60,9 @@ void sock_udp_close(sock_udp_t *sock) netconn_delete(sock->base.conn); sock->base.conn = NULL; } +#ifdef SOCK_HAS_ASYNC_CTX + sock_event_close(sock_udp_get_async_ctx(sock)); +#endif } int sock_udp_get_local(sock_udp_t *sock, sock_udp_ep_t *ep) diff --git a/pkg/openwsn/sock/openwsn_sock_udp.c b/pkg/openwsn/sock/openwsn_sock_udp.c index d2da5fe981..a73d26640d 100644 --- a/pkg/openwsn/sock/openwsn_sock_udp.c +++ b/pkg/openwsn/sock/openwsn_sock_udp.c @@ -349,6 +349,9 @@ void sock_udp_close(sock_udp_t *sock) LL_DELETE(_udp_socket_list, sock); mutex_unlock(&_sock_list_lock); sock->next = NULL; +#ifdef SOCK_HAS_ASYNC_CTX + sock_event_close(sock_udp_get_async_ctx(udp)); +#endif } } diff --git a/pkg/tinydtls/contrib/sock_dtls.c b/pkg/tinydtls/contrib/sock_dtls.c index f03fe6df52..b3912930e4 100644 --- a/pkg/tinydtls/contrib/sock_dtls.c +++ b/pkg/tinydtls/contrib/sock_dtls.c @@ -939,6 +939,9 @@ ssize_t sock_dtls_recv_buf_aux(sock_dtls_t *sock, sock_dtls_session_t *remote, void sock_dtls_close(sock_dtls_t *sock) { dtls_free_context(sock->dtls_ctx); +#ifdef SOCK_HAS_ASYNC_CTX + sock_event_close(sock_dtls_get_async_ctx(sock)); +#endif } void sock_dtls_init(void) diff --git a/sys/include/net/sock/async/event.h b/sys/include/net/sock/async/event.h index 17cac78e64..f0a9600917 100644 --- a/sys/include/net/sock/async/event.h +++ b/sys/include/net/sock/async/event.h @@ -25,6 +25,13 @@ * For the following example [`sock_udp`](@ref net_sock_udp) is used. It is * however easily adaptable for other `sock` types: * + * @warning An async socket may only be closed from the same thread that + * processes the queue. This is because there is no way to prevent the + * networking subsystem from posting socket events other than calling + * sock_*_close(), which would then race against these events. If + * unsure, use sock_*_event_close(), which will close the socket on + * the correct thread. + * * ### An asynchronous UDP Echo server using the event API * * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ {.c} @@ -191,6 +198,15 @@ extern "C" { */ void sock_dtls_event_init(sock_dtls_t *sock, event_queue_t *ev_queue, sock_dtls_cb_t handler, void *handler_arg); + +/** + * @brief Close a possibly async DTLS socket + * + * Helper function that closes a possibly async socket on the event thread. + * + * @param sock socket + */ +void sock_dtls_event_close(sock_dtls_t *sock); #endif /* defined(MODULE_SOCK_DTLS) || defined(DOXYGEN) */ #if defined(MODULE_SOCK_IP) || defined(DOXYGEN) @@ -208,6 +224,15 @@ void sock_dtls_event_init(sock_dtls_t *sock, event_queue_t *ev_queue, */ void sock_ip_event_init(sock_ip_t *sock, event_queue_t *ev_queue, sock_ip_cb_t handler, void *handler_arg); + +/** + * @brief Close a possibly async IP socket + * + * Helper function that closes a possibly async socket on the event thread. + * + * @param sock socket + */ +void sock_ip_event_close(sock_ip_t *sock); #endif /* defined(MODULE_SOCK_IP) || defined(DOXYGEN) */ #if defined(MODULE_SOCK_TCP) || defined(DOXYGEN) @@ -226,6 +251,15 @@ void sock_ip_event_init(sock_ip_t *sock, event_queue_t *ev_queue, void sock_tcp_event_init(sock_tcp_t *sock, event_queue_t *ev_queue, sock_tcp_cb_t handler, void *handler_arg); +/** + * @brief Close a possibly async TCP socket + * + * Helper function that closes a possibly async socket on the event thread. + * + * @param sock socket + */ +void sock_tcp_event_close(sock_tcp_t *sock); + /** * @brief Makes a TCP listening queue able to handle asynchronous events using * @ref sys_event. @@ -257,8 +291,28 @@ void sock_tcp_queue_event_init(sock_tcp_queue_t *queue, event_queue_t *ev_queue, */ void sock_udp_event_init(sock_udp_t *sock, event_queue_t *ev_queue, sock_udp_cb_t handler, void *handler_arg); + +/** + * @brief Close a possibly async UDP socket + * + * Helper function that closes a possibly async socket on the event thread. + * + * @param sock socket + */ +void sock_udp_event_close(sock_udp_t *sock); + #endif /* defined(MODULE_SOCK_UDP) || defined(DOXYGEN) */ +/** + * @brief clear any pending socket async events + * + * @warning Do not call this in the application, it is automatically called by + * sock_*_close(). + * + * @param[in] async_ctx socket async context + */ +void sock_event_close(sock_async_ctx_t *async_ctx); + #ifdef __cplusplus } #endif diff --git a/sys/net/gnrc/sock/ip/gnrc_sock_ip.c b/sys/net/gnrc/sock/ip/gnrc_sock_ip.c index 448f71a875..61fc23deb4 100644 --- a/sys/net/gnrc/sock/ip/gnrc_sock_ip.c +++ b/sys/net/gnrc/sock/ip/gnrc_sock_ip.c @@ -28,6 +28,10 @@ #include "gnrc_sock_internal.h" +#ifdef SOCK_HAS_ASYNC_CTX +#include "net/sock/async/event.h" +#endif + int sock_ip_create(sock_ip_t *sock, const sock_ip_ep_t *local, const sock_ip_ep_t *remote, uint8_t proto, uint16_t flags) { @@ -65,6 +69,9 @@ void sock_ip_close(sock_ip_t *sock) { assert(sock != NULL); gnrc_netreg_unregister(GNRC_NETTYPE_IPV6, &sock->reg.entry); +#ifdef SOCK_HAS_ASYNC_CTX + sock_event_close(sock_ip_get_async_ctx(sock)); +#endif } int sock_ip_get_local(sock_ip_t *sock, sock_ip_ep_t *local) diff --git a/sys/net/gnrc/sock/udp/gnrc_sock_udp.c b/sys/net/gnrc/sock/udp/gnrc_sock_udp.c index 0e494525a2..d5f57d255b 100644 --- a/sys/net/gnrc/sock/udp/gnrc_sock_udp.c +++ b/sys/net/gnrc/sock/udp/gnrc_sock_udp.c @@ -28,6 +28,10 @@ #include "net/udp.h" #include "random.h" +#ifdef SOCK_HAS_ASYNC_CTX +#include "net/sock/async/event.h" +#endif + #include "gnrc_sock_internal.h" #define ENABLE_DEBUG 0 @@ -155,6 +159,9 @@ void sock_udp_close(sock_udp_t *sock) { assert(sock != NULL); gnrc_netreg_unregister(GNRC_NETTYPE_UDP, &sock->reg.entry); +#ifdef SOCK_HAS_ASYNC_CTX + sock_event_close(sock_udp_get_async_ctx(sock)); +#endif #ifdef MODULE_GNRC_SOCK_CHECK_REUSE if (_udp_socks != NULL) { gnrc_sock_reg_t *head = (gnrc_sock_reg_t *)_udp_socks; diff --git a/sys/net/sock/async/event/sock_async_event.c b/sys/net/sock/async/event/sock_async_event.c index 69ed79292c..df4195e5dc 100644 --- a/sys/net/sock/async/event/sock_async_event.c +++ b/sys/net/sock/async/event/sock_async_event.c @@ -62,6 +62,100 @@ static void _set_ctx(sock_async_ctx_t *ctx, event_queue_t *ev_queue) ctx->queue = ev_queue; } +void sock_event_close(sock_async_ctx_t *async_ctx) +{ + event_queue_t *queue = async_ctx->queue; + if (!queue) { + /* no callback registered */ + return; + } + + /* RIOTs socket API is not thread safe so we assume that wherever this is + * called from it's not racing against some other socket usage. + * + * But we have to go stricter and assume this is called from the same thread + * that processes the events. There is no way of preventing the networking + * subsystem from posting socket events until the socket has been + * unregistered, which is only happening when closing the socket and the + * reason we're here in the first place. */ + assume(!queue->waiter || thread_getpid_of(queue->waiter) == thread_getpid()); + + event_cancel(async_ctx->queue, &async_ctx->event.super); +} + +typedef enum { +#if defined(MODULE_SOCK_UDP) || defined(DOXYGEN) + SOCK_ASYNC_UDP, +#endif +#if defined(MODULE_SOCK_TCP) || defined(DOXYGEN) + SOCK_ASYNC_TCP, +#endif +#if defined(MODULE_SOCK_IP) || defined(DOXYGEN) + SOCK_ASYNC_IP, +#endif +#if defined(MODULE_SOCK_DTLS) || defined(DOXYGEN) + SOCK_ASYNC_DTLS, +#endif +} sock_async_t; + +typedef struct { + event_t super; + sock_async_t type; + void *sock; +} _sock_close_ev; + +static void _sock_event_close_cb(event_t *ev) +{ + _sock_close_ev *close_ev = container_of(ev, _sock_close_ev, super); + + switch (close_ev->type) { +#if defined(MODULE_SOCK_UDP) || defined(DOXYGEN) + case SOCK_ASYNC_UDP: + sock_udp_close(close_ev->sock); + break; +#endif +#if defined(MODULE_SOCK_TCP) || defined(DOXYGEN) + case SOCK_ASYNC_TCP: + sock_tcp_disconnect(close_ev->sock); + break; +#endif +#if defined(MODULE_SOCK_IP) || defined(DOXYGEN) + case SOCK_ASYNC_IP: + sock_ip_close(close_ev->sock); + break; +#endif +#if defined(MODULE_SOCK_DTLS) || defined(DOXYGEN) + case SOCK_ASYNC_DTLS: + sock_dtls_close(close_ev->sock); + break; +#endif + } +} + +static void _sock_event_close_common(void *sock, sock_async_ctx_t *async_ctx, + sock_async_t type) +{ + _sock_close_ev ev = { + .super.handler = _sock_event_close_cb, + .type = type, + .sock = sock, + }; + + if (!async_ctx->queue || + !async_ctx->queue->waiter || + thread_getpid_of(async_ctx->queue->waiter) == thread_getpid()) { + /* - this socket is not async OR + * - there is no thread processing the event queue (in which case we + * might race against @ref event_queue_claim() but so is life) + * OR we are on the event queue thread. In the first case we might + * block forever, in the second we surely will, so do it now. */ + _sock_event_close_cb(&ev.super); + return; + } + + event_post(async_ctx->queue, &ev.super); + event_sync(async_ctx->queue); +} #ifdef MODULE_SOCK_DTLS static void _dtls_cb(sock_dtls_t *sock, sock_async_flags_t type, void *arg) { @@ -77,6 +171,11 @@ void sock_dtls_event_init(sock_dtls_t *sock, event_queue_t *ev_queue, ctx->event.cb.dtls = handler; sock_dtls_set_cb(sock, _dtls_cb, handler_arg); } + +void sock_dtls_event_close(sock_dtls_t *sock) +{ + _sock_event_close_common(sock, sock_dtls_get_async_ctx(sock), SOCK_ASYNC_DTLS); +} #endif /* MODULE_SOCK_DTLS */ #ifdef MODULE_SOCK_IP @@ -94,6 +193,11 @@ void sock_ip_event_init(sock_ip_t *sock, event_queue_t *ev_queue, ctx->event.cb.ip = handler; sock_ip_set_cb(sock, _ip_cb, handler_arg); } + +void sock_ip_event_close(sock_ip_t *sock) +{ + _sock_event_close_common(sock, sock_ip_get_async_ctx(sock), SOCK_ASYNC_IP); +} #endif /* MODULE_SOCK_IP */ #ifdef MODULE_SOCK_TCP @@ -127,6 +231,11 @@ void sock_tcp_queue_event_init(sock_tcp_queue_t *queue, event_queue_t *ev_queue, ctx->event.cb.tcp_queue = handler; sock_tcp_queue_set_cb(queue, _tcp_queue_cb, handler_arg); } + +void sock_tcp_event_close(sock_tcp_t *sock) +{ + _sock_event_close_common(sock, sock_tcp_get_async_ctx(sock), SOCK_ASYNC_TCP); +} #endif /* MODULE_SOCK_TCP */ #ifdef MODULE_SOCK_UDP @@ -144,6 +253,12 @@ void sock_udp_event_init(sock_udp_t *sock, event_queue_t *ev_queue, ctx->event.cb.udp = handler; sock_udp_set_cb(sock, _udp_cb, handler_arg); } + +void sock_udp_event_close(sock_udp_t *sock) +{ + _sock_event_close_common(sock, sock_udp_get_async_ctx(sock), SOCK_ASYNC_UDP); +} + #endif /* MODULE_SOCK_UDP */ /** @} */