diff --git a/components/lwip/api/api_msg.c b/components/lwip/api/api_msg.c index d6fbff30e..960548f35 100644 --- a/components/lwip/api/api_msg.c +++ b/components/lwip/api/api_msg.c @@ -710,6 +710,14 @@ netconn_alloc(enum netconn_type t, netconn_callback callback) } #endif +#if ESP_THREAD_SAFE + conn->recvmbox_ref = conn->recvmbox; + sys_mbox_set_owner(&conn->recvmbox, conn); +#if LWIP_TCP + sys_mbox_set_invalid(&conn->acceptmbox_ref); +#endif +#endif + #if LWIP_TCP sys_mbox_set_invalid(&conn->acceptmbox); #endif @@ -760,6 +768,18 @@ netconn_free(struct netconn *conn) !sys_mbox_valid(&conn->acceptmbox)); #endif /* LWIP_TCP */ +#if ESP_THREAD_SAFE + if (conn->recvmbox_ref) { + sys_mbox_free(&conn->recvmbox_ref); + } + +#if LWIP_TCP + if (conn->acceptmbox_ref) { + sys_mbox_free(&conn->acceptmbox_ref); + } +#endif +#endif + #if !LWIP_NETCONN_SEM_PER_THREAD sys_sem_free(&conn->op_completed); sys_sem_set_invalid(&conn->op_completed); @@ -1399,6 +1419,10 @@ lwip_netconn_do_listen(void *m) msg->err = sys_mbox_new(&msg->conn->acceptmbox, DEFAULT_ACCEPTMBOX_SIZE); } if (msg->err == ERR_OK) { +#if ESP_THREAD_SAFE + msg->conn->acceptmbox_ref = msg->conn->acceptmbox; + sys_mbox_set_owner(&msg->conn->acceptmbox, msg->conn); +#endif msg->conn->state = NETCONN_LISTEN; msg->conn->pcb.tcp = lpcb; tcp_arg(msg->conn->pcb.tcp, msg->conn); diff --git a/components/lwip/include/lwip/lwip/api.h b/components/lwip/include/lwip/lwip/api.h index 5b6a21ecf..fa9709122 100644 --- a/components/lwip/include/lwip/lwip/api.h +++ b/components/lwip/include/lwip/lwip/api.h @@ -195,6 +195,15 @@ struct netconn { by the application thread */ sys_mbox_t acceptmbox; #endif /* LWIP_TCP */ + +#if ESP_THREAD_SAFE + /** point to the same mbox as recvmbox */ + sys_mbox_t recvmbox_ref; +#if LWIP_TCP + /** point to the same mbox as acceptmbox */ + sys_mbox_t acceptmbox_ref; +#endif +#endif /** only used for socket layer */ #if LWIP_SOCKET int socket; diff --git a/components/lwip/include/lwip/lwip/sys.h b/components/lwip/include/lwip/lwip/sys.h index 67729e3b7..a7908f60a 100644 --- a/components/lwip/include/lwip/lwip/sys.h +++ b/components/lwip/include/lwip/lwip/sys.h @@ -209,6 +209,17 @@ err_t sys_mbox_trypost(sys_mbox_t *mbox, void *msg); or SYS_ARCH_TIMEOUT on timeout * The returned time has to be accurate to prevent timer jitter! */ u32_t sys_arch_mbox_fetch(sys_mbox_t *mbox, void **msg, u32_t timeout); + +#if ESP_THREAD_SAFE +/** + * @ingroup sys_mbox + * Set the owner of the mbox + * @param mbox mbox to set the owner + * @param owner the owner of the mbox, it's a pointer to struct netconn + */ +void sys_mbox_set_owner(sys_mbox_t *mbox, void *owner); +#endif + /* Allow port to override with a macro, e.g. special timeout for sys_arch_mbox_fetch() */ #ifndef sys_arch_mbox_tryfetch /** Wait for a new message to arrive in the mbox diff --git a/components/lwip/include/lwip/port/arch/sys_arch.h b/components/lwip/include/lwip/port/arch/sys_arch.h index bb7ea18af..f3a0625b8 100644 --- a/components/lwip/include/lwip/port/arch/sys_arch.h +++ b/components/lwip/include/lwip/port/arch/sys_arch.h @@ -50,8 +50,7 @@ typedef xTaskHandle sys_thread_t; typedef struct sys_mbox_s { xQueueHandle os_mbox; - sys_mutex_t lock; - uint8_t alive; + void *owner; }* sys_mbox_t; diff --git a/components/lwip/port/freertos/sys_arch.c b/components/lwip/port/freertos/sys_arch.c index 2c56969ad..671a6f92a 100644 --- a/components/lwip/port/freertos/sys_arch.c +++ b/components/lwip/port/freertos/sys_arch.c @@ -222,16 +222,11 @@ sys_mbox_new(sys_mbox_t *mbox, int size) return ERR_MEM; } - if (sys_mutex_new(&((*mbox)->lock)) != ERR_OK){ - LWIP_DEBUGF(ESP_THREAD_SAFE_DEBUG, ("fail to new *mbox->lock\n")); - vQueueDelete((*mbox)->os_mbox); - free(*mbox); - return ERR_MEM; - } +#if ESP_THREAD_SAFE + (*mbox)->owner = NULL; +#endif - (*mbox)->alive = true; - - LWIP_DEBUGF(ESP_THREAD_SAFE_DEBUG, ("new *mbox ok mbox=%p os_mbox=%p mbox_lock=%p\n", *mbox, (*mbox)->os_mbox, (*mbox)->lock)); + LWIP_DEBUGF(ESP_THREAD_SAFE_DEBUG, ("new *mbox ok mbox=%p os_mbox=%p\n", *mbox, (*mbox)->os_mbox)); return ERR_OK; } @@ -289,42 +284,17 @@ sys_arch_mbox_fetch(sys_mbox_t *mbox, void **msg, u32_t timeout) if (*mbox == NULL){ *msg = NULL; - LWIP_DEBUGF(ESP_THREAD_SAFE_DEBUG, ("sys_arch_mbox_fetch: null mbox\n")); return -1; } - sys_mutex_lock(&(*mbox)->lock); - - if (timeout != 0) { - if (pdTRUE == xQueueReceive((*mbox)->os_mbox, &(*msg), timeout / portTICK_PERIOD_MS)) { - EndTime = xTaskGetTickCount(); - Elapsed = (EndTime - StartTime) * portTICK_PERIOD_MS; - - if (Elapsed == 0) { - Elapsed = 1; - } - - ulReturn = Elapsed; - } else { // timed out blocking for message - *msg = NULL; - ulReturn = SYS_ARCH_TIMEOUT; - } - } else { // block forever for a message. - - while (1){ - LWIP_DEBUGF(ESP_THREAD_SAFE_DEBUG, ("sys_arch_mbox_fetch: fetch mbox=%p os_mbox=%p lock=%p\n", mbox, (*mbox)->os_mbox, (*mbox)->lock)); - if (pdTRUE == xQueueReceive((*mbox)->os_mbox, &(*msg), portMAX_DELAY)){ - LWIP_DEBUGF(ESP_THREAD_SAFE_DEBUG, ("sys_arch_mbox_fetch:mbox rx msg=%p\n", (*msg))); - break; - } - - if ((*mbox)->alive == false){ - LWIP_DEBUGF(ESP_THREAD_SAFE_DEBUG, ("sys_arch_mbox_fetch:mbox not alive\n")); - *msg = NULL; - break; - } - } + if (timeout == 0) { + timeout = portMAX_DELAY; + } else { + timeout = timeout / portTICK_PERIOD_MS; + } + *msg = NULL; + if (pdTRUE == xQueueReceive((*mbox)->os_mbox, &(*msg), timeout)) { EndTime = xTaskGetTickCount(); Elapsed = (EndTime - StartTime) * portTICK_PERIOD_MS; @@ -333,10 +303,10 @@ sys_arch_mbox_fetch(sys_mbox_t *mbox, void **msg, u32_t timeout) } ulReturn = Elapsed; + } else { // timed out blocking for message + ulReturn = SYS_ARCH_TIMEOUT; } - sys_mutex_unlock(&(*mbox)->lock); - return ulReturn ; // return time blocked TBD test } @@ -361,6 +331,16 @@ sys_arch_mbox_tryfetch(sys_mbox_t *mbox, void **msg) } /*-----------------------------------------------------------------------------------*/ + +void +sys_mbox_set_owner(sys_mbox_t *mbox, void* owner) +{ + if (mbox && *mbox) { + (*mbox)->owner = owner; + LWIP_DEBUGF(ESP_THREAD_SAFE_DEBUG, ("set mbox=%p owner=%p", *mbox, owner)); + } +} + /* Deallocates a mailbox. If there are messages still present in the mailbox when the mailbox is deallocated, it is an indication of a @@ -369,50 +349,52 @@ sys_arch_mbox_tryfetch(sys_mbox_t *mbox, void **msg) void sys_mbox_free(sys_mbox_t *mbox) { -#define MAX_POLL_CNT 100 -#define PER_POLL_DELAY 20 - uint16_t count = 0; - bool post_null = true; + uint32_t mbox_message_num = 0; - LWIP_DEBUGF(ESP_THREAD_SAFE_DEBUG, ("sys_mbox_free: set alive false\n")); - (*mbox)->alive = false; + if ( (NULL == mbox) || (NULL == *mbox) ) { + ESP_LOGW(TAG, "WARNING: free null mbox\n"); + return; + } - while ( count++ < MAX_POLL_CNT ){ //ESP32_WORKAROUND - LWIP_DEBUGF(ESP_THREAD_SAFE_DEBUG, ("sys_mbox_free:try lock=%d\n", count)); - if (!sys_mutex_trylock( &(*mbox)->lock )){ - LWIP_DEBUGF(ESP_THREAD_SAFE_DEBUG, ("sys_mbox_free:get lock ok %d\n", count)); - sys_mutex_unlock( &(*mbox)->lock ); - break; - } + mbox_message_num = uxQueueMessagesWaiting((*mbox)->os_mbox); - if (post_null){ - LWIP_DEBUGF(ESP_THREAD_SAFE_DEBUG, ("sys_mbox_free: post null to mbox\n")); - if (sys_mbox_trypost( mbox, NULL) != ERR_OK){ - ESP_STATS_DROP_INC(esp.free_mbox_post_fail); - LWIP_DEBUGF(ESP_THREAD_SAFE_DEBUG, ("sys_mbox_free: post null mbox fail\n")); + LWIP_DEBUGF(ESP_THREAD_SAFE_DEBUG, ("mbox free: mbox=%p os_mbox=%p owner=%p msg_num=%d\n", + *mbox, (*mbox)->os_mbox, (*mbox)->owner, mbox_message_num)); + +#if ESP_THREAD_SAFE + if ((*mbox)->owner) { + if (0 == mbox_message_num) { + /* + * If mbox->owner is not NULL, it indicates the mbox is recvmbox or acceptmbox, + * we need to post a NULL message to mbox in case some application tasks are blocked + * on this mbox + */ + if (sys_mbox_trypost(mbox, NULL) != ERR_OK) { + /* Should never be here because post a message to empty mbox should always be successful */ + ESP_LOGW(TAG, "WARNING: failed to post NULL msg to mbox\n"); } else { - post_null = false; - LWIP_DEBUGF(ESP_THREAD_SAFE_DEBUG, ("sys_mbox_free: post null mbox ok\n")); + LWIP_DEBUGF(ESP_THREAD_SAFE_DEBUG, ("mbox free: post null successfully\n")); } } - - if (count == (MAX_POLL_CNT-1)){ - ESP_LOGW(TAG, "WARNING: mbox %p had a consumer who never unblocked. Leaking!\n", (*mbox)->os_mbox); + (*mbox)->owner = NULL; + } else { + if (mbox_message_num > 1) { + ESP_LOGW(TAG, "WARNING: mbox has %d message, potential memory leaking\n", mbox_message_num); } - sys_delay_ms(PER_POLL_DELAY); + + if (mbox_message_num > 0) { + LWIP_DEBUGF(ESP_THREAD_SAFE_DEBUG, ("mbox free: reset mbox queue\n")); + xQueueReset((*mbox)->os_mbox); + } + + /* For recvmbox or acceptmbox, free them in netconn_free() when all sockets' API are returned */ + vQueueDelete((*mbox)->os_mbox); + free(*mbox); } - - LWIP_DEBUGF(ESP_THREAD_SAFE_DEBUG, ("sys_mbox_free:free mbox\n")); - - if (uxQueueMessagesWaiting((*mbox)->os_mbox)) { - xQueueReset((*mbox)->os_mbox); - /* Line for breakpoint. Should never break here! */ - __asm__ volatile ("nop"); - } - +#else vQueueDelete((*mbox)->os_mbox); - sys_mutex_free(&(*mbox)->lock); free(*mbox); +#endif *mbox = NULL; }