Merge branch 'bugfix/tw27096_fix_lwip_mbox_free_crash_v3.0' into 'release/v3.0'

fix the crash caused by sys_mbox_free (backport v3.0)

See merge request idf/esp-idf!3675
This commit is contained in:
Jiang Jiang Jian 2018-11-15 10:32:21 +08:00
commit 820200e66b
5 changed files with 104 additions and 79 deletions

View file

@ -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);

View file

@ -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;

View file

@ -205,6 +205,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

View file

@ -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;

View file

@ -213,16 +213,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;
}
@ -280,42 +275,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;
@ -324,10 +294,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
}
@ -352,6 +322,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
@ -360,50 +340,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;
}