Skip to content

Commit 2202b2e

Browse files
committed
add AsyncWebServerRequest::clientRelease() method
this will explicitly relese ownership of AsyncClient* object. Make it more clear on ownership change for SSE/WebSocket
1 parent f7ec43d commit 2202b2e

File tree

6 files changed

+96
-38
lines changed

6 files changed

+96
-38
lines changed

src/AsyncEventSource.cpp

Lines changed: 20 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -143,7 +143,7 @@ size_t AsyncEventSourceMessage::send(AsyncClient *client) {
143143

144144
// Client
145145

146-
AsyncEventSourceClient::AsyncEventSourceClient(AsyncWebServerRequest *request, AsyncEventSource *server) : _client(request->client()), _server(server) {
146+
AsyncEventSourceClient::AsyncEventSourceClient(AsyncWebServerRequest *request, AsyncEventSource *server) : _client(request->clientRelease()), _server(server) {
147147

148148
if (request->hasHeader(T_Last_Event_ID)) {
149149
_lastId = atoi(request->getHeader(T_Last_Event_ID)->value().c_str());
@@ -181,9 +181,9 @@ AsyncEventSourceClient::AsyncEventSourceClient(AsyncWebServerRequest *request, A
181181
);
182182

183183
_server->_addClient(this);
184-
delete request;
185-
186184
_client->setNoDelay(true);
185+
// delete AsyncWebServerRequest object (and bound response) since we have the ownership on client connection now
186+
delete request;
187187
}
188188

189189
AsyncEventSourceClient::~AsyncEventSourceClient() {
@@ -470,8 +470,7 @@ void AsyncEventSource::_adjust_inflight_window() {
470470

471471
/* Response */
472472

473-
AsyncEventSourceResponse::AsyncEventSourceResponse(AsyncEventSource *server) {
474-
_server = server;
473+
AsyncEventSourceResponse::AsyncEventSourceResponse(AsyncEventSource *server) : _server(server) {
475474
_code = 200;
476475
_contentType = T_text_event_stream;
477476
_sendContentLength = false;
@@ -482,13 +481,23 @@ AsyncEventSourceResponse::AsyncEventSourceResponse(AsyncEventSource *server) {
482481
void AsyncEventSourceResponse::_respond(AsyncWebServerRequest *request) {
483482
String out;
484483
_assembleHead(out, request->version());
484+
// unbind client's onAck callback from AsyncWebServerRequest's, we will destroy it on next callback and steal the client,
485+
// can't do it now 'cause now we are in AsyncWebServerRequest::_onAck 's stack actually
486+
// here we are loosing time on one RTT delay, but with current design we can't get rid of Req/Resp objects other way
487+
_request = request;
488+
request->client()->onAck(
489+
[](void *r, AsyncClient *c, size_t len, uint32_t time) {
490+
if (len)
491+
static_cast<AsyncEventSourceResponse*>(r)->_switchClient();
492+
},
493+
this
494+
);
485495
request->client()->write(out.c_str(), _headLength);
486496
_state = RESPONSE_WAIT_ACK;
487497
}
488498

489-
size_t AsyncEventSourceResponse::_ack(AsyncWebServerRequest *request, size_t len, uint32_t time __attribute__((unused))) {
490-
if (len) {
491-
new AsyncEventSourceClient(request, _server);
492-
}
493-
return 0;
494-
}
499+
void AsyncEventSourceResponse::_switchClient(){
500+
// AsyncEventSourceClient c-tor will take the ownership of AsyncTCP's client connection
501+
new AsyncEventSourceClient(_request, _server);
502+
// AsyncEventSourceClient c-tor would also delete _request and *this
503+
};

src/AsyncEventSource.h

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -141,6 +141,13 @@ class AsyncEventSourceClient {
141141
void _runQueue();
142142

143143
public:
144+
/**
145+
* @brief Construct a new Async Event Source Client object
146+
* @note constructor would take the ownership of of AsyncTCP's client pointer from `request` parameter and call delete on it!
147+
*
148+
* @param request
149+
* @param server
150+
*/
144151
AsyncEventSourceClient(AsyncWebServerRequest *request, AsyncEventSource *server);
145152
~AsyncEventSourceClient();
146153

@@ -312,11 +319,14 @@ class AsyncEventSource : public AsyncWebHandler {
312319
class AsyncEventSourceResponse : public AsyncWebServerResponse {
313320
private:
314321
AsyncEventSource *_server;
322+
AsyncWebServerRequest *_request;
323+
// this call back will switch AsyncTCP client to SSE
324+
void _switchClient();
315325

316326
public:
317327
AsyncEventSourceResponse(AsyncEventSource *server);
318328
void _respond(AsyncWebServerRequest *request);
319-
size_t _ack(AsyncWebServerRequest *request, size_t len, uint32_t time) override;
329+
size_t _ack(AsyncWebServerRequest *request, size_t len, uint32_t time) override { return 0; };
320330
bool _sourceValid() const {
321331
return true;
322332
}

src/AsyncWebSocket.cpp

Lines changed: 30 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -221,14 +221,16 @@ size_t AsyncWebSocketMessage::send(AsyncClient *client) {
221221
const char *AWSC_PING_PAYLOAD = "ESPAsyncWebServer-PING";
222222
const size_t AWSC_PING_PAYLOAD_LEN = 22;
223223

224-
AsyncWebSocketClient::AsyncWebSocketClient(AsyncWebServerRequest *request, AsyncWebSocket *server) : _tempObject(NULL) {
225-
_client = request->client();
226-
_server = server;
227-
_clientId = _server->_getNextId();
228-
_status = WS_CONNECTED;
229-
_pstate = 0;
230-
_lastMessageTime = millis();
231-
_keepAlivePeriod = 0;
224+
AsyncWebSocketClient::AsyncWebSocketClient(AsyncClient *client, AsyncWebSocket *server)
225+
: _client(client),
226+
_server(server),
227+
_clientId(_server->_getNextId()),
228+
_status(WS_CONNECTED),
229+
_pstate(0),
230+
_lastMessageTime(millis()),
231+
_keepAlivePeriod(0),
232+
_tempObject(NULL) {
233+
232234
_client->setRxTimeout(0);
233235
_client->onError(
234236
[](void *r, AsyncClient *c, int8_t error) {
@@ -272,7 +274,6 @@ AsyncWebSocketClient::AsyncWebSocketClient(AsyncWebServerRequest *request, Async
272274
},
273275
this
274276
);
275-
delete request;
276277
memset(&_pinfo, 0, sizeof(_pinfo));
277278
}
278279

@@ -806,7 +807,9 @@ void AsyncWebSocket::_handleEvent(AsyncWebSocketClient *client, AwsEventType typ
806807

807808
AsyncWebSocketClient *AsyncWebSocket::_newClient(AsyncWebServerRequest *request) {
808809
_clients.emplace_back(request, this);
809-
_handleEvent(&_clients.back(), WS_EVT_CONNECT, request, NULL, 0);
810+
// we've just detached AsyncTCP client from AsyncWebServerRequest,
811+
// *request was destructed along with response object in AsyncWebSocketClient's c-tor
812+
_handleEvent(&_clients.back(), WS_EVT_CONNECT, nullptr /* request */, NULL, 0);
810813
return &_clients.back();
811814
}
812815

@@ -1243,8 +1246,7 @@ AsyncWebSocketMessageBuffer *AsyncWebSocket::makeBuffer(const uint8_t *data, siz
12431246
* Authentication code from https://github.com/Links2004/arduinoWebSockets/blob/master/src/WebSockets.cpp#L480
12441247
*/
12451248

1246-
AsyncWebSocketResponse::AsyncWebSocketResponse(const String &key, AsyncWebSocket *server) {
1247-
_server = server;
1249+
AsyncWebSocketResponse::AsyncWebSocketResponse(const String &key, AsyncWebSocket *server) : _server(server) {
12481250
_code = 101;
12491251
_sendContentLength = false;
12501252

@@ -1290,18 +1292,25 @@ void AsyncWebSocketResponse::_respond(AsyncWebServerRequest *request) {
12901292
request->client()->close();
12911293
return;
12921294
}
1295+
// unbind client's onAck callback from AsyncWebServerRequest's, we will destroy it on next callback and steal the client,
1296+
// can't do it now 'cause now we are in AsyncWebServerRequest::_onAck 's stack actually
1297+
// here we are loosing time on one RTT delay, but with current design we can't get rid of Req/Resp objects other way
1298+
_request = request;
1299+
request->client()->onAck(
1300+
[](void *r, AsyncClient *c, size_t len, uint32_t time) {
1301+
if (len)
1302+
static_cast<AsyncWebSocketResponse*>(r)->_switchClient();
1303+
},
1304+
this
1305+
);
12931306
String out;
12941307
_assembleHead(out, request->version());
12951308
request->client()->write(out.c_str(), _headLength);
12961309
_state = RESPONSE_WAIT_ACK;
12971310
}
12981311

1299-
size_t AsyncWebSocketResponse::_ack(AsyncWebServerRequest *request, size_t len, uint32_t time) {
1300-
(void)time;
1301-
1302-
if (len) {
1303-
_server->_newClient(request);
1304-
}
1305-
1306-
return 0;
1307-
}
1312+
void AsyncWebSocketResponse::_switchClient(){
1313+
// detach client from request
1314+
_server->_newClient(_request);
1315+
// _newClient() would also destruct _request and *this
1316+
}

src/AsyncWebSocket.h

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -211,18 +211,18 @@ class AsyncWebSocketClient {
211211
AsyncWebSocket *_server;
212212
uint32_t _clientId;
213213
AwsClientStatus _status;
214+
uint8_t _pstate;
215+
uint32_t _lastMessageTime;
216+
uint32_t _keepAlivePeriod;
214217
#ifdef ESP32
215218
mutable std::recursive_mutex _lock;
216219
#endif
217220
std::deque<AsyncWebSocketControl> _controlQueue;
218221
std::deque<AsyncWebSocketMessage> _messageQueue;
219222
bool closeWhenFull = true;
220223

221-
uint8_t _pstate;
222224
AwsFrameInfo _pinfo;
223225

224-
uint32_t _lastMessageTime;
225-
uint32_t _keepAlivePeriod;
226226

227227
bool _queueControl(uint8_t opcode, const uint8_t *data = NULL, size_t len = 0, bool mask = false);
228228
bool _queueMessage(AsyncWebSocketSharedBuffer buffer, uint8_t opcode = WS_TEXT, bool mask = false);
@@ -232,7 +232,15 @@ class AsyncWebSocketClient {
232232
public:
233233
void *_tempObject;
234234

235-
AsyncWebSocketClient(AsyncWebServerRequest *request, AsyncWebSocket *server);
235+
AsyncWebSocketClient(AsyncClient *client, AsyncWebSocket *server);
236+
237+
/**
238+
* @brief Construct a new Async Web Socket Client object
239+
* @note constructor would take the ownership of of AsyncTCP's client pointer from `request` parameter and call delete on it!
240+
* @param request
241+
* @param server
242+
*/
243+
AsyncWebSocketClient(AsyncWebServerRequest *request, AsyncWebSocket *server) : AsyncWebSocketClient(request->clientRelease(), server) { delete request; };
236244
~AsyncWebSocketClient();
237245

238246
// client id increments for the given server
@@ -464,11 +472,14 @@ class AsyncWebSocketResponse : public AsyncWebServerResponse {
464472
private:
465473
String _content;
466474
AsyncWebSocket *_server;
475+
AsyncWebServerRequest *_request;
476+
// this call back will switch AsyncTCP client to WebSocket
477+
void _switchClient();
467478

468479
public:
469480
AsyncWebSocketResponse(const String &key, AsyncWebSocket *server);
470481
void _respond(AsyncWebServerRequest *request);
471-
size_t _ack(AsyncWebServerRequest *request, size_t len, uint32_t time) override;
482+
size_t _ack(AsyncWebServerRequest *request, size_t len, uint32_t time) override { return 0; };
472483
bool _sourceValid() const {
473484
return true;
474485
}

src/ESPAsyncWebServer.h

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -305,6 +305,19 @@ class AsyncWebServerRequest {
305305
AsyncClient *client() {
306306
return _client;
307307
}
308+
309+
/**
310+
* @brief release owned AsyncClient object
311+
* AsyncClient pointer will be abandoned in this instance,
312+
* the further ownership of the connection should be managed out of request's life-time scope
313+
* could be used for long lived connection like SSE or WebSockets
314+
* @note do not call this methos unless you know what you are doing, otherwise it may lead to
315+
* memory leaks and connections lingering
316+
*
317+
* @return AsyncClient* pointer to released connection object
318+
*/
319+
AsyncClient* clientRelease();
320+
308321
uint8_t version() const {
309322
return _version;
310323
}

src/WebRequest.cpp

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1153,3 +1153,9 @@ bool AsyncWebServerRequest::isExpectedRequestedConnType(RequestedConnectionType
11531153
return ((erct1 != RCT_NOT_USED) && (erct1 == _reqconntype)) || ((erct2 != RCT_NOT_USED) && (erct2 == _reqconntype))
11541154
|| ((erct3 != RCT_NOT_USED) && (erct3 == _reqconntype));
11551155
}
1156+
1157+
AsyncClient* AsyncWebServerRequest::clientRelease(){
1158+
AsyncClient* c = _client;
1159+
_client = nullptr;
1160+
return c;
1161+
}

0 commit comments

Comments
 (0)