From 5bbb5d5b25e0011a762ad3dabd1ee7a2ecac2f7a Mon Sep 17 00:00:00 2001 From: Hamza Hajeir Date: Thu, 3 Oct 2024 14:06:07 +0300 Subject: [PATCH 1/3] fix(asyncudp): Fixes and implements tcpip thread locking --- libraries/AsyncUDP/src/AsyncUDP.cpp | 36 ++++++++++++++--------------- 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/libraries/AsyncUDP/src/AsyncUDP.cpp b/libraries/AsyncUDP/src/AsyncUDP.cpp index 4f799b4d4a4..669b2b96a87 100644 --- a/libraries/AsyncUDP/src/AsyncUDP.cpp +++ b/libraries/AsyncUDP/src/AsyncUDP.cpp @@ -15,6 +15,19 @@ extern "C" { #include "lwip/priv/tcpip_priv.h" +#ifdef CONFIG_LWIP_TCPIP_CORE_LOCKING +#define UDP_MUTEX_LOCK() if (!sys_thread_tcpip(LWIP_CORE_LOCK_QUERY_HOLDER)) { \ + LOCK_TCPIP_CORE(); \ + } + +#define UDP_MUTEX_UNLOCK() if (sys_thread_tcpip(LWIP_CORE_LOCK_QUERY_HOLDER)) { \ + UNLOCK_TCPIP_CORE(); \ + } +#else // CONFIG_LWIP_TCPIP_CORE_LOCKING +#define UDP_MUTEX_LOCK() +#define UDP_MUTEX_UNLOCK() +#endif // CONFIG_LWIP_TCPIP_CORE_LOCKING + static const char *netif_ifkeys[TCPIP_ADAPTER_IF_MAX] = {"WIFI_STA_DEF", "WIFI_AP_DEF", "ETH_DEF", "PPP_DEF"}; static esp_err_t tcpip_adapter_get_netif(tcpip_adapter_if_t tcpip_if, void **netif) { @@ -28,7 +41,9 @@ static esp_err_t tcpip_adapter_get_netif(tcpip_adapter_if_t tcpip_if, void **net if (netif_index < 0) { return ESP_FAIL; } + UDP_MUTEX_LOCK(); *netif = (void *)netif_get_by_index(netif_index); + UDP_MUTEX_UNLOCK(); } else { *netif = netif_default; } @@ -232,9 +247,6 @@ static bool _udp_task_stop(){ } */ -#define UDP_MUTEX_LOCK() //xSemaphoreTake(_lock, portMAX_DELAY) -#define UDP_MUTEX_UNLOCK() //xSemaphoreGive(_lock) - AsyncUDPMessage::AsyncUDPMessage(size_t size) { _index = 0; if (size > CONFIG_TCP_MSS) { @@ -473,12 +485,13 @@ bool AsyncUDP::_init() { if (_pcb) { return true; } + UDP_MUTEX_LOCK(); _pcb = udp_new(); if (!_pcb) { return false; } - //_lock = xSemaphoreCreateMutex(); udp_recv(_pcb, &_udp_recv, (void *)this); + UDP_MUTEX_UNLOCK(); return true; } @@ -493,14 +506,12 @@ AsyncUDP::~AsyncUDP() { close(); UDP_MUTEX_LOCK(); udp_recv(_pcb, NULL, NULL); + UDP_MUTEX_UNLOCK(); _udp_remove(_pcb); _pcb = NULL; - UDP_MUTEX_UNLOCK(); - //vSemaphoreDelete(_lock); } void AsyncUDP::close() { - UDP_MUTEX_LOCK(); if (_pcb != NULL) { if (_connected) { _udp_disconnect(_pcb); @@ -508,7 +519,6 @@ void AsyncUDP::close() { _connected = false; //todo: unjoin multicast group } - UDP_MUTEX_UNLOCK(); } bool AsyncUDP::connect(const ip_addr_t *addr, uint16_t port) { @@ -520,14 +530,11 @@ bool AsyncUDP::connect(const ip_addr_t *addr, uint16_t port) { return false; } close(); - UDP_MUTEX_LOCK(); _lastErr = _udp_connect(_pcb, addr, port); if (_lastErr != ERR_OK) { - UDP_MUTEX_UNLOCK(); return false; } _connected = true; - UDP_MUTEX_UNLOCK(); return true; } @@ -544,13 +551,10 @@ bool AsyncUDP::listen(const ip_addr_t *addr, uint16_t port) { IP_SET_TYPE_VAL(_pcb->local_ip, addr->type); IP_SET_TYPE_VAL(_pcb->remote_ip, addr->type); } - UDP_MUTEX_LOCK(); if (_udp_bind(_pcb, addr, port) != ERR_OK) { - UDP_MUTEX_UNLOCK(); return false; } _connected = true; - UDP_MUTEX_UNLOCK(); return true; } @@ -624,12 +628,10 @@ bool AsyncUDP::listenMulticast(const ip_addr_t *addr, uint16_t port, uint8_t ttl return false; } - UDP_MUTEX_LOCK(); _pcb->mcast_ttl = ttl; _pcb->remote_port = port; ip_addr_copy(_pcb->remote_ip, *addr); //ip_addr_copy(_pcb->remote_ip, ip_addr_any_type); - UDP_MUTEX_UNLOCK(); return true; } @@ -651,7 +653,6 @@ size_t AsyncUDP::writeTo(const uint8_t *data, size_t len, const ip_addr_t *addr, if (pbt != NULL) { uint8_t *dst = reinterpret_cast(pbt->payload); memcpy(dst, data, len); - UDP_MUTEX_LOCK(); if (tcpip_if < TCPIP_ADAPTER_IF_MAX) { void *nif = NULL; tcpip_adapter_get_netif((tcpip_adapter_if_t)tcpip_if, &nif); @@ -663,7 +664,6 @@ size_t AsyncUDP::writeTo(const uint8_t *data, size_t len, const ip_addr_t *addr, } else { _lastErr = _udp_sendto(_pcb, pbt, addr, port); } - UDP_MUTEX_UNLOCK(); pbuf_free(pbt); if (_lastErr < ERR_OK) { return 0; From be56836d35b5d7718ec2e0004a055234d588b8d6 Mon Sep 17 00:00:00 2001 From: Hamza Hajeir Date: Fri, 4 Oct 2024 14:19:02 +0300 Subject: [PATCH 2/3] fix(asyncudp): Adds missing unlock --- libraries/AsyncUDP/src/AsyncUDP.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/libraries/AsyncUDP/src/AsyncUDP.cpp b/libraries/AsyncUDP/src/AsyncUDP.cpp index 669b2b96a87..f3604a4a866 100644 --- a/libraries/AsyncUDP/src/AsyncUDP.cpp +++ b/libraries/AsyncUDP/src/AsyncUDP.cpp @@ -488,6 +488,7 @@ bool AsyncUDP::_init() { UDP_MUTEX_LOCK(); _pcb = udp_new(); if (!_pcb) { + UDP_MUTEX_UNLOCK(); return false; } udp_recv(_pcb, &_udp_recv, (void *)this); From 848b0aeaf3b41c1baff1a5cd7e9696b62e8d0748 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci-lite[bot]" <117423508+pre-commit-ci-lite[bot]@users.noreply.github.com> Date: Mon, 7 Oct 2024 13:59:34 +0000 Subject: [PATCH 3/3] ci(pre-commit): Apply automatic fixes --- libraries/AsyncUDP/src/AsyncUDP.cpp | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/libraries/AsyncUDP/src/AsyncUDP.cpp b/libraries/AsyncUDP/src/AsyncUDP.cpp index f3604a4a866..5549276de44 100644 --- a/libraries/AsyncUDP/src/AsyncUDP.cpp +++ b/libraries/AsyncUDP/src/AsyncUDP.cpp @@ -16,17 +16,19 @@ extern "C" { #include "lwip/priv/tcpip_priv.h" #ifdef CONFIG_LWIP_TCPIP_CORE_LOCKING -#define UDP_MUTEX_LOCK() if (!sys_thread_tcpip(LWIP_CORE_LOCK_QUERY_HOLDER)) { \ - LOCK_TCPIP_CORE(); \ - } - -#define UDP_MUTEX_UNLOCK() if (sys_thread_tcpip(LWIP_CORE_LOCK_QUERY_HOLDER)) { \ - UNLOCK_TCPIP_CORE(); \ - } -#else // CONFIG_LWIP_TCPIP_CORE_LOCKING +#define UDP_MUTEX_LOCK() \ + if (!sys_thread_tcpip(LWIP_CORE_LOCK_QUERY_HOLDER)) { \ + LOCK_TCPIP_CORE(); \ + } + +#define UDP_MUTEX_UNLOCK() \ + if (sys_thread_tcpip(LWIP_CORE_LOCK_QUERY_HOLDER)) { \ + UNLOCK_TCPIP_CORE(); \ + } +#else // CONFIG_LWIP_TCPIP_CORE_LOCKING #define UDP_MUTEX_LOCK() #define UDP_MUTEX_UNLOCK() -#endif // CONFIG_LWIP_TCPIP_CORE_LOCKING +#endif // CONFIG_LWIP_TCPIP_CORE_LOCKING static const char *netif_ifkeys[TCPIP_ADAPTER_IF_MAX] = {"WIFI_STA_DEF", "WIFI_AP_DEF", "ETH_DEF", "PPP_DEF"};