Skip to content
This repository was archived by the owner on Jan 20, 2025. It is now read-only.

Commit 8ee20ad

Browse files
committed
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.
1 parent 8607ac1 commit 8ee20ad

1 file changed

Lines changed: 11 additions & 3 deletions

File tree

src/AsyncTCP.cpp

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,9 @@ void _asynctcpsock_task(void *)
8888

8989
xSemaphoreTakeRecursive(_asyncsock_mutex, (TickType_t)portMAX_DELAY);
9090

91-
// Collect all of the active sockets into socket set
91+
// Collect all of the active sockets into socket set. Half-destroyed
92+
// connections should have set _socket to -1 and therefore should not
93+
// end up in the sockList.
9294
FD_ZERO(&sockSet_r); FD_ZERO(&sockSet_w);
9395
for (it = _socketBaseList.begin(); it != _socketBaseList.end(); it++) {
9496
if ((*it)->_socket != -1) {
@@ -123,7 +125,9 @@ void _asynctcpsock_task(void *)
123125
// Check all sockets to see which ones are active
124126
uint32_t nActive = 0;
125127
if (r > 0) {
126-
// Collect and notify all writable sockets
128+
// Collect and notify all writable sockets. Half-destroyed connections
129+
// should have set _socket to -1 and therefore should not end up in
130+
// the sockList.
127131
for (it = _socketBaseList.begin(); it != _socketBaseList.end(); it++) {
128132
if ((*it)->_selected && FD_ISSET((*it)->_socket, &sockSet_w)) {
129133
sockList.push_back(*it);
@@ -147,7 +151,9 @@ void _asynctcpsock_task(void *)
147151
}
148152
sockList.clear();
149153

150-
// Collect and notify all readable sockets
154+
// Collect and notify all readable sockets. Half-destroyed connections
155+
// should have set _socket to -1 and therefore should not end up in
156+
// the sockList.
151157
for (it = _socketBaseList.begin(); it != _socketBaseList.end(); it++) {
152158
if ((*it)->_selected && FD_ISSET((*it)->_socket, &sockSet_r)) {
153159
sockList.push_back(*it);
@@ -294,10 +300,12 @@ AsyncClient::AsyncClient(int sockfd)
294300

295301
AsyncClient::~AsyncClient()
296302
{
303+
xSemaphoreTakeRecursive(_asyncsock_mutex, (TickType_t)portMAX_DELAY);
297304
if (_socket != -1) _close();
298305
_removeAllCallbacks();
299306
vSemaphoreDelete(_write_mutex);
300307
_write_mutex = NULL;
308+
xSemaphoreGiveRecursive(_asyncsock_mutex);
301309
}
302310

303311
void AsyncClient::setRxTimeout(uint32_t timeout){

0 commit comments

Comments
 (0)