From 931757f4a9524a2bd0959f6442aad01878f5e567 Mon Sep 17 00:00:00 2001
From: Scott Shawcroft <scott@tannewt.org>
Date: Tue, 26 Jul 2022 15:29:49 -0700
Subject: [PATCH] Improve web workflow responsiveness

1. Run the socket select task at the same priority as CP. This is
  needed because it queues up the background work. Without it, CP
  needed to sleep to let the lower priority task go.
2. Close the active socket on disconnect. This prevents looping
  over a disconnected but not closed socket.

Fixes #6610. Fixes #6613
---
 .../espressif/common-hal/socketpool/Socket.c  | 51 ++++++++++---------
 supervisor/shared/web_workflow/web_workflow.c |  3 ++
 2 files changed, 30 insertions(+), 24 deletions(-)

diff --git a/ports/espressif/common-hal/socketpool/Socket.c b/ports/espressif/common-hal/socketpool/Socket.c
index 45d5f1fea0..f6025d329c 100644
--- a/ports/espressif/common-hal/socketpool/Socket.c
+++ b/ports/espressif/common-hal/socketpool/Socket.c
@@ -60,32 +60,35 @@ STATIC void socket_select_task(void *arg) {
         FD_SET(socket_change_fd, &errfds);
         int max_fd = socket_change_fd;
         for (size_t i = 0; i < MP_ARRAY_SIZE(open_socket_fds); i++) {
-            if (open_socket_fds[i] < 0) {
+            int sockfd = open_socket_fds[i];
+            if (sockfd < 0) {
                 continue;
             }
-            max_fd = MAX(max_fd, open_socket_fds[i]);
-            FD_SET(open_socket_fds[i], &readfds);
-            FD_SET(open_socket_fds[i], &errfds);
+            max_fd = MAX(max_fd, sockfd);
+            FD_SET(sockfd, &readfds);
+            FD_SET(sockfd, &errfds);
         }
 
         int num_triggered = select(max_fd + 1, &readfds, NULL, &errfds, NULL);
-        if (num_triggered < 0) {
-            // Maybe bad file descriptor
-            for (size_t i = 0; i < MP_ARRAY_SIZE(open_socket_fds); i++) {
-                int sockfd = open_socket_fds[i];
-                if (sockfd < 0) {
-                    continue;
-                }
-                if (FD_ISSET(sockfd, &errfds)) {
-                    int err;
-                    int optlen = sizeof(int);
-                    int ret = getsockopt(sockfd, SOL_SOCKET, SO_ERROR, &err, (socklen_t *)&optlen);
-                    if (ret < 0) {
-                        open_socket_fds[i] = -1;
-                        // Try again.
-                        continue;
-                    }
-                }
+        // Check for bad file descriptor and queue up the background task before
+        // circling around.
+        if (num_triggered == -1 && errno == EBADF) {
+            // One for the change fd and one for the closed socket.
+            num_triggered = 2;
+        }
+        // Try and find the bad file and remove it from monitoring.
+        for (size_t i = 0; i < MP_ARRAY_SIZE(open_socket_fds); i++) {
+            int sockfd = open_socket_fds[i];
+            if (sockfd < 0) {
+                continue;
+            }
+            int err;
+            int optlen = sizeof(int);
+            int ret = getsockopt(sockfd, SOL_SOCKET, SO_ERROR, &err, (socklen_t *)&optlen);
+            if (ret < 0) {
+                open_socket_fds[i] = -1;
+                // Raise num_triggered so that we skip the assert and queue the background task.
+                num_triggered = 2;
             }
         }
         assert(num_triggered >= 0);
@@ -117,13 +120,13 @@ void socket_user_reset(void) {
             user_socket[i] = false;
         }
         socket_change_fd = eventfd(0, 0);
-        // This task runs at a lower priority than CircuitPython and is used to wake CircuitPython
-        // up when any open sockets have data to read. It allows us to sleep otherwise.
+        // Run this at the same priority as CP so that the web workflow background task can be
+        // queued while CP is running. Both tasks can still sleep and, therefore, sleep overall.
         (void)xTaskCreateStaticPinnedToCore(socket_select_task,
             "socket_select",
             2 * configMINIMAL_STACK_SIZE,
             NULL,
-            0, // Run this at IDLE priority. We only need it when CP isn't running (at 1).
+            uxTaskPriorityGet(NULL),
             socket_select_stack,
             &socket_select_task_handle,
             xPortGetCoreID());
diff --git a/supervisor/shared/web_workflow/web_workflow.c b/supervisor/shared/web_workflow/web_workflow.c
index 1fb8cdeff9..ff1c4060c3 100644
--- a/supervisor/shared/web_workflow/web_workflow.c
+++ b/supervisor/shared/web_workflow/web_workflow.c
@@ -1265,6 +1265,9 @@ void supervisor_web_workflow_background(void) {
     // If we have a request in progress, continue working on it.
     if (common_hal_socketpool_socket_get_connected(&active)) {
         _process_request(&active, &active_request);
+    } else {
+        // Close the active socket if it is no longer connected.
+        common_hal_socketpool_socket_close(&active);
     }
 }