]> granicus.if.org Git - esp-idf/commitdiff
lwip: fix mbox thread-safe issue
authorLiu Zhi Fu <liuzhifu@espressif.com>
Thu, 20 Dec 2018 06:03:11 +0000 (14:03 +0800)
committerLiu Zhi Fu <liuzhifu@espressif.com>
Wed, 2 Jan 2019 14:48:19 +0000 (22:48 +0800)
Fix a mbox free thread-safe issue that can lead to crash in sys_arch_mbox_fetch.

components/lwip/lwip
components/lwip/port/esp32/freertos/sys_arch.c
components/lwip/port/esp32/include/arch/sys_arch.h

index 0865edf80ad0a96e0eaf2bbedbad350cab248115..e2a24de6603c896a5ddacac296c0304002376eae 160000 (submodule)
@@ -1 +1 @@
-Subproject commit 0865edf80ad0a96e0eaf2bbedbad350cab248115
+Subproject commit e2a24de6603c896a5ddacac296c0304002376eae
index 4270f957b3124e0c818ebcb2758e47405a1fcb39..1168b9e89ec72684e051ed768c9f48fc4e2f8d03 100644 (file)
@@ -352,7 +352,6 @@ sys_mbox_free(sys_mbox_t *mbox)
   uint32_t mbox_message_num = 0;
 
   if ( (NULL == mbox) || (NULL == *mbox) ) {
-      ESP_LOGW(TAG, "WARNING: free null mbox\n");
       return;
   }
 
@@ -390,12 +389,13 @@ sys_mbox_free(sys_mbox_t *mbox)
     /* For recvmbox or acceptmbox, free them in netconn_free() when all sockets' API are returned */
     vQueueDelete((*mbox)->os_mbox);
     free(*mbox);
+    *mbox = NULL;
   }
 #else
   vQueueDelete((*mbox)->os_mbox);
   free(*mbox);
-#endif
   *mbox = NULL;
+#endif
 }
 
 /*-----------------------------------------------------------------------------------*/
index bfa2ad6caa1100330517a950231c6b6410d7ab99..9638fdfd621e7384c42cc891bca57ba7b5054656 100644 (file)
@@ -62,7 +62,25 @@ typedef struct sys_mbox_s {
 #endif
 
 #define sys_mbox_valid( x ) ( ( ( *x ) == NULL) ? pdFALSE : pdTRUE )
-#define sys_mbox_set_invalid( x ) ( ( *x ) = NULL )
+
+/* Define the sys_mbox_set_invalid() to empty to support lock-free mbox in ESP LWIP.
+ * 
+ * The basic idea about the lock-free mbox is that the mbox should always be valid unless
+ * no socket APIs are using the socket and the socket is closed. ESP LWIP achieves this by
+ * following two changes to official LWIP:
+ * 1. Postpone the deallocation of mbox to netconn_free(), in other words, free the mbox when
+ *    no one is using the socket.
+ * 2. Define the sys_mbox_set_invalid() to empty if the mbox is not actually freed.
+
+ * The second change is necessary. Consider a common scenario: the application task calls 
+ * recv() to receive packets from the socket, the sys_mbox_valid() returns true. Because there
+ * is no lock for the mbox, the LWIP CORE can call sys_mbox_set_invalid() to set the mbox at 
+ * anytime and the thread-safe issue may happen.
+ *
+ * However, if the sys_mbox_set_invalid() is not called after sys_mbox_free(), e.g. in netconn_alloc(),
+ * we need to initialize the mbox to invalid explicitly since sys_mbox_set_invalid() now is empty.
+ */
+#define sys_mbox_set_invalid( x ) 
 
 #define sys_sem_valid( x ) ( ( ( *x ) == NULL) ? pdFALSE : pdTRUE )
 #define sys_sem_set_invalid( x ) ( ( *x ) = NULL )