From a5ae72c0f6069b67f9eb0e5ed22282c6bbba2474 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Alex=20Villac=C3=ADs=20Lasso?= <a_villacis@palosanto.com>
Date: Mon, 6 Sep 2021 16:03:31 -0500
Subject: [PATCH] Properly use nonzero select() timeout for writable sockets

A socket that is "at rest" (no pending reads or writes) is by definition
"writable", and using select() to check for writability will return TRUE
 for the socket without any delay. Therefore each active connection must
 be checked for pending data to write before being added to the write
 check set. This fixes failure to yield to other tasks due to a busy
 loop.
---
 src/AsyncTCP.cpp | 28 +++++++++++++++++-----------
 src/AsyncTCP.h   |  3 +++
 2 files changed, 20 insertions(+), 11 deletions(-)

diff --git a/src/AsyncTCP.cpp b/src/AsyncTCP.cpp
index fef63cc..4146c6c 100644
--- a/src/AsyncTCP.cpp
+++ b/src/AsyncTCP.cpp
@@ -93,7 +93,9 @@ void _asynctcpsock_task(void *)
         for (it = _socketBaseList.begin(); it != _socketBaseList.end(); it++) {
             if ((*it)->_socket != -1) {
                 FD_SET((*it)->_socket, &sockSet_r);
-                FD_SET((*it)->_socket, &sockSet_w);
+                if ((*it)->_pendingWrite()) {
+                    FD_SET((*it)->_socket, &sockSet_w);
+                }
                 (*it)->_selected = true;
                 if (max_sock <= (*it)->_socket) max_sock = (*it)->_socket + 1;
             }
@@ -102,10 +104,15 @@ void _asynctcpsock_task(void *)
         // Wait for activity on all monitored sockets
         struct timeval tv;
         tv.tv_sec = 0;
-        tv.tv_usec = 0;
+        tv.tv_usec = ASYNCTCPSOCK_POLL_INTERVAL * 1000;
         uint32_t t1 = millis();
+
+        xSemaphoreGiveRecursive(_asyncsock_mutex);
+
         int r = select(max_sock, &sockSet_r, &sockSet_w, NULL, &tv);
 
+        xSemaphoreTakeRecursive(_asyncsock_mutex, (TickType_t)portMAX_DELAY);
+
         // Check all sockets to see which ones are active
         uint32_t nActive = 0;
         if (r > 0) {
@@ -182,15 +189,6 @@ void _asynctcpsock_task(void *)
 
         xSemaphoreGiveRecursive(_asyncsock_mutex);
 
-        uint32_t t2 = millis();
-
-        // Ugly hack to work around the apparent situation of select() call NOT
-        // yielding to other tasks if using a nonzero wait period.
-        uint32_t d = (nActive == 0 && t2 - t1 < ASYNCTCPSOCK_POLL_INTERVAL)
-            ? ASYNCTCPSOCK_POLL_INTERVAL - (t2 - t1)
-            : 1;
-        delay(d);
-
         // Collect and run activity poll on all pollable sockets
         xSemaphoreTakeRecursive(_asyncsock_mutex, (TickType_t)portMAX_DELAY);
         for (it = _socketBaseList.begin(); it != _socketBaseList.end(); it++) {
@@ -865,6 +863,14 @@ bool AsyncClient::send()
     return true;
 }
 
+bool AsyncClient::_pendingWrite(void)
+{
+    xSemaphoreTake(_write_mutex, (TickType_t)portMAX_DELAY);
+    bool pending = (_writeQueue.size() > 0);
+    xSemaphoreGive(_write_mutex);
+    return pending;
+}
+
 // In normal operation this should be a no-op. Will only free something in case
 // of errors before all data was written.
 void AsyncClient::_clearWriteQueue(void)
diff --git a/src/AsyncTCP.h b/src/AsyncTCP.h
index 6c24749..0b70a07 100644
--- a/src/AsyncTCP.h
+++ b/src/AsyncTCP.h
@@ -70,6 +70,8 @@ protected:
     virtual void _sockPoll(void) {}           // Action to take on idle socket activity poll
     virtual void _sockDelayedConnect(void) {} // Action to take on DNS-resolve finished
 
+    virtual bool _pendingWrite(void) { return false; }  // Test if there is data pending to be written
+
 public:
     AsyncSocketBase(void);
     virtual ~AsyncSocketBase();
@@ -142,6 +144,7 @@ class AsyncClient : public AsyncSocketBase
     void _sockIsReadable(void);
     void _sockPoll(void);
     void _sockDelayedConnect(void);
+    bool _pendingWrite(void);
 
   private: