From 8ee20adb2bbc159a2e5155953fd455090eaf9fbf Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Alex=20Villac=C3=ADs=20Lasso?= <a_villacis@palosanto.com>
Date: Wed, 8 Sep 2021 12:53:22 -0500
Subject: [PATCH] Protect against AsyncClient being destroyed in the middle of
 processing

- Take the _asyncsock_mutex in AsyncClient destructor. This should
  handle scenarios where a previously-valid client is destroyed by
  another task while being examined by the asyncTcpSock task.
- Since the _asyncsock_mutex is released before being taken again by the
  AsyncSocketBase destructor, examined objects may be still "half-destroyed"
  (AsyncClient destructor terminated and _write_mutex invalid, but still
   waiting for lock in AsyncSocketBase destructor). However, _socket may
   now be guaranteed to  remain valid if it was valid when the
   _asyncsock_mutex was taken.
---
 src/AsyncTCP.cpp | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/src/AsyncTCP.cpp b/src/AsyncTCP.cpp
index 66acd2f..2a06f34 100644
--- a/src/AsyncTCP.cpp
+++ b/src/AsyncTCP.cpp
@@ -88,7 +88,9 @@ void _asynctcpsock_task(void *)
 
         xSemaphoreTakeRecursive(_asyncsock_mutex, (TickType_t)portMAX_DELAY);
 
-        // Collect all of the active sockets into socket set
+        // Collect all of the active sockets into socket set. Half-destroyed
+        // connections should have set _socket to -1 and therefore should not
+        // end up in the sockList.
         FD_ZERO(&sockSet_r); FD_ZERO(&sockSet_w);
         for (it = _socketBaseList.begin(); it != _socketBaseList.end(); it++) {
             if ((*it)->_socket != -1) {
@@ -123,7 +125,9 @@ void _asynctcpsock_task(void *)
         // Check all sockets to see which ones are active
         uint32_t nActive = 0;
         if (r > 0) {
-            // Collect and notify all writable sockets
+            // Collect and notify all writable sockets. Half-destroyed connections
+            // should have set _socket to -1 and therefore should not end up in
+            // the sockList.
             for (it = _socketBaseList.begin(); it != _socketBaseList.end(); it++) {
                 if ((*it)->_selected && FD_ISSET((*it)->_socket, &sockSet_w)) {
                     sockList.push_back(*it);
@@ -147,7 +151,9 @@ void _asynctcpsock_task(void *)
             }
             sockList.clear();
 
-            // Collect and notify all readable sockets
+            // Collect and notify all readable sockets. Half-destroyed connections
+            // should have set _socket to -1 and therefore should not end up in
+            // the sockList.
             for (it = _socketBaseList.begin(); it != _socketBaseList.end(); it++) {
                 if ((*it)->_selected && FD_ISSET((*it)->_socket, &sockSet_r)) {
                     sockList.push_back(*it);
@@ -294,10 +300,12 @@ AsyncClient::AsyncClient(int sockfd)
 
 AsyncClient::~AsyncClient()
 {
+    xSemaphoreTakeRecursive(_asyncsock_mutex, (TickType_t)portMAX_DELAY);
     if (_socket != -1) _close();
     _removeAllCallbacks();
     vSemaphoreDelete(_write_mutex);
     _write_mutex = NULL;
+    xSemaphoreGiveRecursive(_asyncsock_mutex);
 }
 
 void AsyncClient::setRxTimeout(uint32_t timeout){