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
This commit is contained in:
Scott Shawcroft 2022-07-26 15:29:49 -07:00
parent bc926b041e
commit 931757f4a9
No known key found for this signature in database
GPG Key ID: 0DFD512649C052DA
2 changed files with 30 additions and 24 deletions

View File

@ -60,32 +60,35 @@ STATIC void socket_select_task(void *arg) {
FD_SET(socket_change_fd, &errfds); FD_SET(socket_change_fd, &errfds);
int max_fd = socket_change_fd; int max_fd = socket_change_fd;
for (size_t i = 0; i < MP_ARRAY_SIZE(open_socket_fds); i++) { 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; continue;
} }
max_fd = MAX(max_fd, open_socket_fds[i]); max_fd = MAX(max_fd, sockfd);
FD_SET(open_socket_fds[i], &readfds); FD_SET(sockfd, &readfds);
FD_SET(open_socket_fds[i], &errfds); FD_SET(sockfd, &errfds);
} }
int num_triggered = select(max_fd + 1, &readfds, NULL, &errfds, NULL); int num_triggered = select(max_fd + 1, &readfds, NULL, &errfds, NULL);
if (num_triggered < 0) { // Check for bad file descriptor and queue up the background task before
// Maybe bad file descriptor // circling around.
for (size_t i = 0; i < MP_ARRAY_SIZE(open_socket_fds); i++) { if (num_triggered == -1 && errno == EBADF) {
int sockfd = open_socket_fds[i]; // One for the change fd and one for the closed socket.
if (sockfd < 0) { num_triggered = 2;
continue; }
} // Try and find the bad file and remove it from monitoring.
if (FD_ISSET(sockfd, &errfds)) { for (size_t i = 0; i < MP_ARRAY_SIZE(open_socket_fds); i++) {
int err; int sockfd = open_socket_fds[i];
int optlen = sizeof(int); if (sockfd < 0) {
int ret = getsockopt(sockfd, SOL_SOCKET, SO_ERROR, &err, (socklen_t *)&optlen); continue;
if (ret < 0) { }
open_socket_fds[i] = -1; int err;
// Try again. int optlen = sizeof(int);
continue; 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); assert(num_triggered >= 0);
@ -117,13 +120,13 @@ void socket_user_reset(void) {
user_socket[i] = false; user_socket[i] = false;
} }
socket_change_fd = eventfd(0, 0); socket_change_fd = eventfd(0, 0);
// This task runs at a lower priority than CircuitPython and is used to wake CircuitPython // Run this at the same priority as CP so that the web workflow background task can be
// up when any open sockets have data to read. It allows us to sleep otherwise. // queued while CP is running. Both tasks can still sleep and, therefore, sleep overall.
(void)xTaskCreateStaticPinnedToCore(socket_select_task, (void)xTaskCreateStaticPinnedToCore(socket_select_task,
"socket_select", "socket_select",
2 * configMINIMAL_STACK_SIZE, 2 * configMINIMAL_STACK_SIZE,
NULL, 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_stack,
&socket_select_task_handle, &socket_select_task_handle,
xPortGetCoreID()); xPortGetCoreID());

View File

@ -1265,6 +1265,9 @@ void supervisor_web_workflow_background(void) {
// If we have a request in progress, continue working on it. // If we have a request in progress, continue working on it.
if (common_hal_socketpool_socket_get_connected(&active)) { if (common_hal_socketpool_socket_get_connected(&active)) {
_process_request(&active, &active_request); _process_request(&active, &active_request);
} else {
// Close the active socket if it is no longer connected.
common_hal_socketpool_socket_close(&active);
} }
} }