VFS: Allocate socket select semaphore outside ISR

components/lwip/port/esp32/vfs_lwip.c

@@ -28,18 +28,26 @@
 
 _Static_assert(MAX_FDS >= CONFIG_LWIP_MAX_SOCKETS, "MAX_FDS < CONFIG_LWIP_MAX_SOCKETS");
 
-static void lwip_stop_socket_select()
+static void lwip_stop_socket_select(void *sem)
 {
-    sys_sem_signal(sys_thread_sem_get()); //socket_select will return
+    sys_sem_signal(sem); //socket_select will return
 }
 
-static void lwip_stop_socket_select_isr(BaseType_t *woken)
+static void lwip_stop_socket_select_isr(void *sem, BaseType_t *woken)
 {
-    if (sys_sem_signal_isr(sys_thread_sem_get()) && woken) {
+    if (sys_sem_signal_isr(sem) && woken) {
         *woken = pdTRUE;
     }
 }
 
+static void *lwip_get_socket_select_semaphore()
+{
+    /* Calling this from the same process as select() will ensure that the semaphore won't be allocated from
+     * ISR (lwip_stop_socket_select_isr).
+     */
+    return (void *) sys_thread_sem_get();
+}
+
 static int lwip_fcntl_r_wrapper(int fd, int cmd, int arg)
 {
     return lwip_fcntl_r(fd, cmd, arg);
@@ -62,6 +70,7 @@ void esp_vfs_lwip_sockets_register()
         .fcntl = &lwip_fcntl_r_wrapper,
         .ioctl = &lwip_ioctl_r_wrapper,
         .socket_select = &lwip_select,
+        .get_socket_select_semaphore = &lwip_get_socket_select_semaphore,
         .stop_socket_select = &lwip_stop_socket_select,
         .stop_socket_select_isr = &lwip_stop_socket_select_isr,
     };

components/vfs/include/esp_vfs.h

@@ -66,6 +66,16 @@ extern "C" {
  */
 typedef int esp_vfs_id_t;
 
+/**
+ * @brief VFS semaphore type for select()
+ *
+ */
+typedef struct
+{
+    bool is_sem_local;      /*!< type of "sem" is SemaphoreHandle_t when true, defined by socket driver otherwise */
+    void *sem;              /*!< semaphore instance */
+} esp_vfs_select_sem_t;
+
 /**
  * @brief VFS definition structure
  *
@@ -218,14 +228,16 @@ typedef struct
 #endif // CONFIG_SUPPORT_TERMIOS
 
     /** start_select is called for setting up synchronous I/O multiplexing of the desired file descriptors in the given VFS */
-    esp_err_t (*start_select)(int nfds, fd_set *readfds, fd_set *writefds, fd_set *exceptfds, SemaphoreHandle_t *signal_sem);
+    esp_err_t (*start_select)(int nfds, fd_set *readfds, fd_set *writefds, fd_set *exceptfds, esp_vfs_select_sem_t sem);
     /** socket select function for socket FDs with the functionality of POSIX select(); this should be set only for the socket VFS */
     int (*socket_select)(int nfds, fd_set *readfds, fd_set *writefds, fd_set *errorfds, struct timeval *timeout);
     /** called by VFS to interrupt the socket_select call when select is activated from a non-socket VFS driver; set only for the socket driver */
-    void (*stop_socket_select)();
+    void (*stop_socket_select)(void *sem);
     /** stop_socket_select which can be called from ISR; set only for the socket driver */
-    void (*stop_socket_select_isr)(BaseType_t *woken);
+    void (*stop_socket_select_isr)(void *sem, BaseType_t *woken);
     /** end_select is called to stop the I/O multiplexing and deinitialize the environment created by start_select for the given VFS */
+    void* (*get_socket_select_semaphore)();
+    /** get_socket_select_semaphore returns semaphore allocated in the socket driver; set only for the socket driver */
     void (*end_select)();
 } esp_vfs_t;
 
@@ -371,9 +383,9 @@ int esp_vfs_select(int nfds, fd_set *readfds, fd_set *writefds, fd_set *errorfds
  * This function is called when the VFS driver detects a read/write/error
  * condition as it was requested by the previous call to start_select.
  *
- * @param signal_sem semaphore handle which was passed to the driver by the start_select call
+ * @param sem semaphore structure which was passed to the driver by the start_select call
  */
-void esp_vfs_select_triggered(SemaphoreHandle_t *signal_sem);
+void esp_vfs_select_triggered(esp_vfs_select_sem_t sem);
 
 /**
  * @brief Notification from a VFS driver about a read/write/error condition (ISR version)
@@ -381,10 +393,10 @@ void esp_vfs_select_triggered(SemaphoreHandle_t *signal_sem);
  * This function is called when the VFS driver detects a read/write/error
  * condition as it was requested by the previous call to start_select.
  *
- * @param signal_sem semaphore handle which was passed to the driver by the start_select call
+ * @param sem semaphore structure which was passed to the driver by the start_select call
  * @param woken is set to pdTRUE if the function wakes up a task with higher priority
  */
-void esp_vfs_select_triggered_isr(SemaphoreHandle_t *signal_sem, BaseType_t *woken);
+void esp_vfs_select_triggered_isr(esp_vfs_select_sem_t sem, BaseType_t *woken);
 
 /**
  * @brief Implements the VFS layer for synchronous I/O multiplexing by poll()

components/vfs/vfs.c

@@ -824,6 +824,11 @@ int esp_vfs_select(int nfds, fd_set *readfds, fd_set *writefds, fd_set *errorfds
         return -1;
     }
 
+    esp_vfs_select_sem_t sel_sem = {
+        .is_sem_local = false,
+        .sem = NULL,
+    };
+
     int (*socket_select)(int, fd_set *, fd_set *, fd_set *, struct timeval *) = NULL;
     for (int fd = 0; fd < nfds; ++fd) {
         _lock_acquire(&s_fd_table_lock);
@@ -844,6 +849,7 @@ int esp_vfs_select(int nfds, fd_set *readfds, fd_set *writefds, fd_set *errorfds
                         esp_vfs_safe_fd_isset(fd, errorfds)) {
                     const vfs_entry_t *vfs = s_vfs[vfs_index];
                     socket_select = vfs->vfs.socket_select;
+                    sel_sem.sem = vfs->vfs.get_socket_select_semaphore();
                 }
             }
             continue;
@@ -874,19 +880,14 @@ int esp_vfs_select(int nfds, fd_set *readfds, fd_set *writefds, fd_set *errorfds
     // the global readfds, writefds and errorfds contain only socket FDs (if
     // there any)
 
-    /* Semaphore used for waiting select events from other VFS drivers when socket
-     * select is not used (not registered or socket FDs are not observed by the
-     * given call of select)
-     */
-    SemaphoreHandle_t select_sem = NULL;
-
     if (!socket_select) {
         // There is no socket VFS registered or select() wasn't called for
         // any socket. Therefore, we will use our own signalization.
-        if ((select_sem = xSemaphoreCreateBinary()) == NULL) {
+        sel_sem.is_sem_local = true;
+        if ((sel_sem.sem = xSemaphoreCreateBinary()) == NULL) {
             free(vfs_fds_triple);
             __errno_r(r) = ENOMEM;
-            ESP_LOGD(TAG, "cannot create select_sem");
+            ESP_LOGD(TAG, "cannot create select semaphore");
             return -1;
         }
     }
@@ -902,18 +903,18 @@ int esp_vfs_select(int nfds, fd_set *readfds, fd_set *writefds, fd_set *errorfds
             esp_vfs_log_fd_set("readfds", &item->readfds);
             esp_vfs_log_fd_set("writefds", &item->writefds);
             esp_vfs_log_fd_set("errorfds", &item->errorfds);
-            esp_err_t err = vfs->vfs.start_select(nfds, &item->readfds, &item->writefds, &item->errorfds, &select_sem);
+            esp_err_t err = vfs->vfs.start_select(nfds, &item->readfds, &item->writefds, &item->errorfds, sel_sem);
 
             if (err != ESP_OK) {
                 call_end_selects(i, vfs_fds_triple);
                 (void) set_global_fd_sets(vfs_fds_triple, s_vfs_count, readfds, writefds, errorfds);
-                if (select_sem) {
-                    vSemaphoreDelete(select_sem);
-                    select_sem = NULL;
+                if (sel_sem.is_sem_local && sel_sem.sem) {
+                    vSemaphoreDelete(sel_sem.sem);
+                    sel_sem.sem = NULL;
                 }
                 free(vfs_fds_triple);
                 __errno_r(r) = EINTR;
-                ESP_LOGD(TAG, "start_select failed");
+                ESP_LOGD(TAG, "start_select failed: %s", esp_err_to_name(err));
                 return -1;
             }
         }
@@ -947,16 +948,16 @@ int esp_vfs_select(int nfds, fd_set *readfds, fd_set *writefds, fd_set *errorfds
             ESP_LOGD(TAG, "timeout is %dms", timeout_ms);
         }
         ESP_LOGD(TAG, "waiting without calling socket_select");
-        xSemaphoreTake(select_sem, ticks_to_wait);
+        xSemaphoreTake(sel_sem.sem, ticks_to_wait);
     }
 
     call_end_selects(s_vfs_count, vfs_fds_triple); // for VFSs for start_select was called before
     if (ret >= 0) {
         ret += set_global_fd_sets(vfs_fds_triple, s_vfs_count, readfds, writefds, errorfds);
     }
-    if (select_sem) {
-        vSemaphoreDelete(select_sem);
-        select_sem = NULL;
+    if (sel_sem.is_sem_local && sel_sem.sem) {
+        vSemaphoreDelete(sel_sem.sem);
+        sel_sem.sem = NULL;
     }
     free(vfs_fds_triple);
 
@@ -967,10 +968,10 @@ int esp_vfs_select(int nfds, fd_set *readfds, fd_set *writefds, fd_set *errorfds
     return ret;
 }
 
-void esp_vfs_select_triggered(SemaphoreHandle_t *signal_sem)
+void esp_vfs_select_triggered(esp_vfs_select_sem_t sem)
 {
-    if (signal_sem && (*signal_sem)) {
-        xSemaphoreGive(*signal_sem);
+    if (sem.is_sem_local) {
+        xSemaphoreGive(sem.sem);
     } else {
         // Another way would be to go through s_fd_table and find the VFS
         // which has a permanent FD. But in order to avoid to lock
@@ -978,17 +979,17 @@ void esp_vfs_select_triggered(SemaphoreHandle_t *signal_sem)
         for (int i = 0; i < s_vfs_count; ++i) {
             const vfs_entry_t *vfs = s_vfs[i];
             if (vfs != NULL && vfs->vfs.stop_socket_select != NULL) {
-                vfs->vfs.stop_socket_select();
+                vfs->vfs.stop_socket_select(sem.sem);
                 break;
             }
         }
     }
 }
 
-void esp_vfs_select_triggered_isr(SemaphoreHandle_t *signal_sem, BaseType_t *woken)
+void esp_vfs_select_triggered_isr(esp_vfs_select_sem_t sem, BaseType_t *woken)
 {
-    if (signal_sem && (*signal_sem)) {
-        xSemaphoreGiveFromISR(*signal_sem, woken);
+    if (sem.is_sem_local) {
+        xSemaphoreGiveFromISR(sem.sem, woken);
     } else {
         // Another way would be to go through s_fd_table and find the VFS
         // which has a permanent FD. But in order to avoid to lock
@@ -996,7 +997,7 @@ void esp_vfs_select_triggered_isr(SemaphoreHandle_t *signal_sem, BaseType_t *wok
         for (int i = 0; i < s_vfs_count; ++i) {
             const vfs_entry_t *vfs = s_vfs[i];
             if (vfs != NULL && vfs->vfs.stop_socket_select_isr != NULL) {
-                vfs->vfs.stop_socket_select_isr(woken);
+                vfs->vfs.stop_socket_select_isr(sem.sem, woken);
                 break;
             }
         }

components/vfs/vfs_uart.c

@@ -62,7 +62,7 @@ static bool s_non_blocking[UART_NUM];
 /* Lock ensuring that uart_select is used from only one task at the time */
 static _lock_t s_one_select_lock;
 
-static SemaphoreHandle_t *_signal_sem = NULL;
+static esp_vfs_select_sem_t _select_sem = {.sem = NULL};
 static fd_set *_readfds = NULL;
 static fd_set *_writefds = NULL;
 static fd_set *_errorfds = NULL;
@@ -319,25 +319,25 @@ static void select_notif_callback(uart_port_t uart_num, uart_select_notif_t uart
         case UART_SELECT_READ_NOTIF:
             if (FD_ISSET(uart_num, _readfds_orig)) {
                 FD_SET(uart_num, _readfds);
-                esp_vfs_select_triggered_isr(_signal_sem, task_woken);
+                esp_vfs_select_triggered_isr(_select_sem, task_woken);
             }
             break;
         case UART_SELECT_WRITE_NOTIF:
             if (FD_ISSET(uart_num, _writefds_orig)) {
                 FD_SET(uart_num, _writefds);
-                esp_vfs_select_triggered_isr(_signal_sem, task_woken);
+                esp_vfs_select_triggered_isr(_select_sem, task_woken);
             }
             break;
         case UART_SELECT_ERROR_NOTIF:
             if (FD_ISSET(uart_num, _errorfds_orig)) {
                 FD_SET(uart_num, _errorfds);
-                esp_vfs_select_triggered_isr(_signal_sem, task_woken);
+                esp_vfs_select_triggered_isr(_select_sem, task_woken);
             }
             break;
     }
 }
 
-static esp_err_t uart_start_select(int nfds, fd_set *readfds, fd_set *writefds, fd_set *exceptfds, SemaphoreHandle_t *signal_sem)
+static esp_err_t uart_start_select(int nfds, fd_set *readfds, fd_set *writefds, fd_set *exceptfds, esp_vfs_select_sem_t select_sem)
 {
     if (_lock_try_acquire(&s_one_select_lock)) {
         return ESP_ERR_INVALID_STATE;
@@ -347,7 +347,7 @@ static esp_err_t uart_start_select(int nfds, fd_set *readfds, fd_set *writefds,
 
     portENTER_CRITICAL(uart_get_selectlock());
 
-    if (_readfds || _writefds || _errorfds || _readfds_orig || _writefds_orig || _errorfds_orig || _signal_sem) {
+    if (_readfds || _writefds || _errorfds || _readfds_orig || _writefds_orig || _errorfds_orig || _select_sem.sem) {
         portEXIT_CRITICAL(uart_get_selectlock());
         uart_end_select();
         return ESP_ERR_INVALID_STATE;
@@ -378,7 +378,7 @@ static esp_err_t uart_start_select(int nfds, fd_set *readfds, fd_set *writefds,
         }
     }
 
-    _signal_sem = signal_sem;
+    _select_sem = select_sem;
 
     _readfds = readfds;
     _writefds = writefds;
@@ -398,7 +398,7 @@ static esp_err_t uart_start_select(int nfds, fd_set *readfds, fd_set *writefds,
             if (uart_get_buffered_data_len(i, &buffered_size) == ESP_OK && buffered_size > 0) {
                 // signalize immediately when data is buffered
                 FD_SET(i, _readfds);
-                esp_vfs_select_triggered(_signal_sem);
+                esp_vfs_select_triggered(_select_sem);
             }
         }
     }
@@ -417,7 +417,7 @@ static void uart_end_select()
         uart_set_select_notif_callback(i, NULL);
     }
 
-    _signal_sem = NULL;
+    _select_sem.sem = NULL;
 
     _readfds = NULL;
     _writefds = NULL;