diff --git a/docs/eventsource.md b/docs/eventsource.md index 3d5dd86a5..22a7abbe5 100644 --- a/docs/eventsource.md +++ b/docs/eventsource.md @@ -49,6 +49,56 @@ authMiddleware.generateHash(); events.addMiddleware(&authMiddleware); ``` +### Track and close a single client + +Each SSE client is exposed as a shared handle (`AsyncEventSourceClientPtr`, a `std::shared_ptr`). A handle may be stored past the end of any callback: it keeps the client object alive until the last handle is dropped, and a client that disconnects simply reports `connected() == false` instead of becoming a dangling pointer. + +The request-aware connect callback lets you associate the connection with request-scoped state (such as the authenticated user) while the request is still alive: + +```cpp +std::map> sessions; // app-side map + +events.onConnectWithRequest([](AsyncWebServerRequest *request, const AsyncEventSourceClientPtr &client) { + // read authentication/session state from request here + String user = getUserFrom(request); + // store a weak reference: an expired session must not keep a dead client object alive + sessions[user] = client; +}); + +// later, from your normal service context (e.g. session expiry): +if (auto client = sessions["bob"].lock()) { + client->close(); // closes ONLY this connection, other clients stay connected +} + +// send a message to every connected client (safe snapshot): +for (const auto &client : events.getClients()) { + if (client->connected()) { + client->send("update", "state"); + } +} +``` + +**Note**: if you access such app-side containers from several tasks, guard them with a mutex of your own — the connect/disconnect callbacks run on the AsyncTCP task. + +### Ownership-safe callbacks + +Besides the classic `onConnect` / `onDisconnect` callbacks (raw pointer, valid only inside the callback), ownership-safe variants deliver the shared handle and are safe to store: + +```cpp +events.onConnectSafe([](const AsyncEventSourceClientPtr &client) { + // client->send(...) works as usual, and the handle may be retained +}); + +events.onDisconnectSafe([](const AsyncEventSourceClientPtr &client) { + // safe to call methods: the object is kept alive by the handle + Serial.printf("SSE client gone, lastId=%u\n", client->lastId()); +}); +``` + +Each connect/disconnect event has a single callback slot shared by the classic and the safe setter: registering one overwrites the other (the last registered wins). Pick one flavor per event. + +`shared_from_this()` (inherited from `std::enable_shared_from_this`) provides the same handle from a client pointer you already hold, e.g. inside the classic `onConnect` callback — do not call it from the client constructor. + ### Setup Event Source in the browser ```javascript diff --git a/examples/arduino/ServerSentEvents/ServerSentEvents.ino b/examples/arduino/ServerSentEvents/ServerSentEvents.ino index bc6718c8c..765767d8b 100644 --- a/examples/arduino/ServerSentEvents/ServerSentEvents.ino +++ b/examples/arduino/ServerSentEvents/ServerSentEvents.ino @@ -75,6 +75,11 @@ void setup() { client->send("hello!", NULL, millis(), 1000); }); + // ownership-safe variant: the handle may be stored past the callback + events.onConnectSafe([](const AsyncEventSourceClientPtr &client) { + Serial.printf("SSE Client handle %p connected!\n", (void *)client.get()); + }); + events.onDisconnect([](AsyncEventSourceClient *client) { Serial.printf("SSE Client disconnected!"); }); diff --git a/src/AsyncEventSource.cpp b/src/AsyncEventSource.cpp index 26d4e49fb..5311be8e1 100644 --- a/src/AsyncEventSource.cpp +++ b/src/AsyncEventSource.cpp @@ -5,8 +5,10 @@ #include "AsyncWebServerLogging.h" #include +#include #include #include +#include #define ASYNC_SSE_NEW_LINE_CHAR (char)0xa @@ -102,48 +104,70 @@ static String generateEventMessage(const char *message, const char *event, uint3 // Client AsyncEventSourceClient::AsyncEventSourceClient(AsyncClient *client, AsyncEventSource *server, uint32_t lastId) - : _client(client), _server(server), _lastId(lastId) { + : _client(client), _server(server), _lastId(lastId) {} +void AsyncEventSourceClient::_attach() { _client->setRxTimeout(0); _client->onError(NULL, NULL); _client->onAck( [](void *r, AsyncClient *c, size_t len, uint32_t time) { (void)c; - static_cast(r)->_onAck(len, time); + // Pin the client for the duration of the callback so a disconnect + // inside AsyncTCP cannot free it while this frame still runs. + AsyncEventSourceClientPtr self = static_cast(r)->shared_from_this(); + self->_onAck(len, time); }, this ); _client->onPoll( [](void *r, AsyncClient *c) { (void)c; - static_cast(r)->_onPoll(); + AsyncEventSourceClientPtr self = static_cast(r)->shared_from_this(); + self->_onPoll(); }, this ); _client->onData(NULL, NULL); _client->onTimeout( - [this](void *r, AsyncClient *c __attribute__((unused)), uint32_t time) { - static_cast(r)->_onTimeout(time); + [](void *r, AsyncClient *c, uint32_t time) { + (void)c; + AsyncEventSourceClientPtr self = static_cast(r)->shared_from_this(); + self->_onTimeout(time); }, this ); _client->onDisconnect( - [this](void *r, AsyncClient *c) { - static_cast(r)->_onDisconnect(); + [](void *r, AsyncClient *c) { + // Pin first: _onDisconnect() may end up dropping the last server-side + // reference; the client must survive until this frame returns, and + // the AsyncClient delete must happen on this AsyncTCP callback frame. + AsyncEventSourceClientPtr self = static_cast(r)->shared_from_this(); + self->_onDisconnect(); delete c; }, this ); - - _server->_addClient(this); _client->setNoDelay(true); } AsyncEventSourceClient::~AsyncEventSourceClient() { // Protect message queue access (size checks and modifications) which is not thread-safe. - asyncsrv::lock_guard_type lock(_lockmq); - _messageQueue.clear(); - close(); + { + asyncsrv::lock_guard_type lock(_lockmq); + _messageQueue.clear(); + } + if (_client) { + // Still connected: the last handle was dropped (or the server was torn + // down) before the TCP connection ended. Close it directly, but detach the + // AsyncTCP callbacks first -- shared_from_this() is invalid inside a + // destructor, so no callback may re-enter this object while it dies. + _client->onDisconnect({}, nullptr); + _client->onAck({}, nullptr); + _client->onPoll({}, nullptr); + _client->onTimeout({}, nullptr); + delete _client; // ~AsyncClient closes the connection + _client = nullptr; + } } bool AsyncEventSourceClient::_queueMessage(AsyncEvent_SharedData_t &&msg) { @@ -210,8 +234,25 @@ void AsyncEventSourceClient::_onDisconnect() { if (!_client) { return; } - _client = nullptr; - _server->_handleDisconnect(this); + { + // Every queue method (_queueMessage, _runQueue, _onAck, _onPoll) reads + // _client while holding _lockmq; for those guarded reads to be + // meaningful, this write must be synchronized the same way. + asyncsrv::lock_guard_type lock(_lockmq); + _client = nullptr; + } + if (_server) { + _server->_handleDisconnect(this); + } + // Drop queued messages: they can never be delivered anymore. Do it after + // the disconnect callback so packetsWaiting() still reports the backlog + // there, as it did when the client was destroyed immediately on erase. + // Without this, an app holding a handle would keep the backlog in memory + // until the last handle is dropped. + { + asyncsrv::lock_guard_type lock(_lockmq); + _messageQueue.clear(); + } } void AsyncEventSourceClient::close() { @@ -220,6 +261,20 @@ void AsyncEventSourceClient::close() { } } +void AsyncEventSourceClient::_detach() { + _server = nullptr; + if (_client) { + // Detach AsyncTCP callbacks first so the close cannot re-enter this + // object after the owning server is gone. + _client->onDisconnect({}, nullptr); + _client->onAck({}, nullptr); + _client->onPoll({}, nullptr); + _client->onTimeout({}, nullptr); + delete _client; // ~AsyncClient closes the connection + _client = nullptr; + } +} + bool AsyncEventSourceClient::send(const char *message, const char *event, uint32_t id, uint32_t reconnect) { if (!connected()) { return false; @@ -286,30 +341,97 @@ void AsyncEventSourceClient::set_max_inflight_bytes(size_t value) { /* AsyncEventSource */ +AsyncEventSource::~AsyncEventSource() { + close(); + // Detach any client whose handle an application may still hold: the server + // object is going away, so clients must not touch it anymore. New clients + // cannot appear past this point: the handler is removed from the server + // before destruction begins. + std::list leftovers; + { + asyncsrv::lock_guard_type lock(_client_queue_lock); + leftovers.splice(leftovers.end(), _clients); + } + for (const auto &c : leftovers) { + c->_detach(); + } +} + +// wrap the raw-pointer flavor into the shared-handle slot; std::function owns +// its callable, so an anonymous temporary registered by the user stays alive +void AsyncEventSource::onConnect(ArEventHandlerFunction cb) { + if (cb) { + _connectsafecb = [cb](const AsyncEventSourceClientPtr &client) { + cb(client.get()); + }; + } else { + _connectsafecb = nullptr; + } +} + +void AsyncEventSource::onDisconnect(ArEventHandlerFunction cb) { + if (cb) { + _disconnectsafecb = [cb](const AsyncEventSourceClientPtr &client) { + cb(client.get()); + }; + } else { + _disconnectsafecb = nullptr; + } +} + void AsyncEventSource::authorizeConnect(ArAuthorizeConnectHandler cb) { AsyncAuthorizationMiddleware *m = new AsyncAuthorizationMiddleware(401, cb); m->_freeOnRemoval = true; addMiddleware(m); } -void AsyncEventSource::_addClient(AsyncEventSourceClient *client) { +void AsyncEventSource::_addClient(const AsyncEventSourceClientPtr &client, AsyncWebServerRequest *request) { if (!client) { return; } - - if (_connectcb) { - _connectcb(client); + if (!client->_client && request) { + // Adopt the connection from the request. The client was pre-allocated + // (before any bytes were written to the socket) in AsyncEventSourceResponse::_respond(). + client->_client = request->clientRelease(); + } + if (!client->_client) { + // No connection to adopt (e.g. the request was already gone): drop the + // half-constructed client instead of registering a zombie. + return; } - asyncsrv::lock_guard_type lock(_client_queue_lock); - _clients.emplace_back(client); + // Arm the AsyncTCP callbacks under the list lock, together with the list + // insertion: the disconnect callback is armed by _attach() itself + // (clientRelease() cleared them beforehand), so a disconnect racing this + // section can only run after we release the lock -- and _handleDisconnect() + // then finds the client in the list and erases it cleanly. Attaching and + // inserting in separate critical sections would let a disconnect run in + // between and erase from an empty list, leaving a dead entry behind. + // Callbacks are attached here (not in the constructor) because they rely on + // shared_from_this(), which requires shared_ptr ownership. + { + asyncsrv::lock_guard_type lock(_client_queue_lock); + client->_attach(); + _clients.emplace_back(client); + _adjust_inflight_window(); + } - _adjust_inflight_window(); + // Fire user callbacks outside the list lock, after registration: a + // broadcast or close inside a connect callback now behaves consistently + // (the client is part of the list while its callback runs). + if (_connectsafecb) { + _connectsafecb(client); + } + if (_requestConnectcb) { + _requestConnectcb(request, client); + } } void AsyncEventSource::_handleDisconnect(AsyncEventSourceClient *client) { - if (_disconnectcb) { - _disconnectcb(client); + // The client is kept alive by the caller's shared handle (network callback + // pin or app-held handle), so firing callbacks with a usable object is safe. + if (_disconnectsafecb) { + _disconnectsafecb(client->shared_from_this()); } asyncsrv::lock_guard_type lock(_client_queue_lock); for (auto i = _clients.begin(); i != _clients.end(); ++i) { @@ -322,22 +444,33 @@ void AsyncEventSource::_handleDisconnect(AsyncEventSourceClient *client) { } void AsyncEventSource::close() { - // While the whole loop is not done, the linked list is locked and so the - // iterator should remain valid even when AsyncEventSource::_handleDisconnect() - // is called very early - asyncsrv::lock_guard_type lock(_client_queue_lock); - for (const auto &c : _clients) { + // Snapshot the list under the lock, close outside it: a synchronous + // _onDisconnect() inside c->close() erases list nodes, which would + // otherwise invalidate the loop iterator (and could deadlock). + std::vector live; + { + asyncsrv::lock_guard_type lock(_client_queue_lock); + for (const auto &c : _clients) { + live.push_back(c); + } + } + for (const auto &c : live) { if (c->connected()) { - /** - * @brief: Fix self-deadlock by using recursive_mutex instead. - * Due to c->close() shall call the callback function _onDisconnect() - * The calling flow _onDisconnect() --> _handleDisconnect() --> deadlock - */ c->close(); } } } +std::vector AsyncEventSource::getClients() const { + std::vector snapshot; + asyncsrv::lock_guard_type lock(_client_queue_lock); + snapshot.reserve(_clients.size()); + for (const auto &c : _clients) { + snapshot.push_back(c); + } + return snapshot; +} + // pmb fix size_t AsyncEventSource::avgPacketsWaiting() const { size_t aql = 0; @@ -433,8 +566,20 @@ void AsyncEventSourceResponse::_respond(AsyncWebServerRequest *request) { if (request->hasHeader(T_Last_Event_ID)) { lastId = strtoul(request->getHeader(T_Last_Event_ID)->value().c_str(), nullptr, 10); } + // Allocate the SSE client before committing anything to the connection, so + // an allocation failure can still be answered with 503 on the raw HTTP + // connection (after clientRelease() the request can no longer respond). + // The connection itself is adopted from the request inside _addClient(). + AsyncEventSourceClientPtr client(new (std::nothrow) AsyncEventSourceClient(nullptr, _server, lastId)); + if (!client) { + async_ws_log_e("Failed to allocate"); + _state = RESPONSE_FAILED; + request->sendRawResponse(asyncsrv::T_HTTP_503_HEAD); + return; + } request->client()->write(out.c_str(), _headLength, ASYNC_WRITE_FLAG_COPY); - // Add a new AsyncEventSourceClient to the server's list of clients - // This adopts the ownership of the AsyncTCP's client pointer from `request` parameter - new AsyncEventSourceClient(request->clientRelease(), _server, lastId); + // Hold the request in scope for the connect callback to inspect + // (clientRelease() inside _addClient() drops the request's self-reference) + std::shared_ptr reqLock = request->shared_from_this(); + _server->_addClient(client, request); } diff --git a/src/AsyncEventSource.h b/src/AsyncEventSource.h index f01c58980..8b08a91c1 100644 --- a/src/AsyncEventSource.h +++ b/src/AsyncEventSource.h @@ -46,11 +46,18 @@ #include #include #include +#include class AsyncEventSource; class AsyncEventSourceResponse; class AsyncEventSourceClient; +// Ownership-safe handle to an SSE client connection. +// The handle keeps the client object alive even after it disconnected, and can +// be retained past any callback; use it as a stable connection identifier. +using AsyncEventSourceClientPtr = std::shared_ptr; using ArEventHandlerFunction = std::function; +using ArEventHandlerFunctionSafe = std::function; +using ArEventRequestConnectHandlerFunction = std::function; using ArAuthorizeConnectHandler = ArAuthorizeFunction; // shared message object container using AsyncEvent_SharedData_t = std::shared_ptr; @@ -59,8 +66,9 @@ using AsyncEvent_SharedData_t = std::shared_ptr; * @brief class holds a sse messages queue for a particular client's connection * */ -class AsyncEventSourceClient { +class AsyncEventSourceClient : public std::enable_shared_from_this { private: + friend class AsyncEventSource; AsyncClient *_client; AsyncEventSource *_server; uint32_t _lastId{0}; @@ -72,11 +80,19 @@ class AsyncEventSourceClient { mutable asyncsrv::mutex_type _lockmq; bool _queueMessage(AsyncEvent_SharedData_t &&msg); void _runQueue(); + // bind AsyncTCP callbacks (ack/poll/timeout/disconnect) to this client; + // called by AsyncEventSource::_addClient() once the client is shared-owned + void _attach(); + // detach from the owning server (used when the server dies before the client); + // clears AsyncTCP callbacks and closes the connection + void _detach(); public: /** * @brief Construct a new Async Event Source Client object * @note constructor is normally passed a client object from AsyncWebServerRequest::releaseClient(); see AsyncEventSourceResponse::_respond() + * @note the client does not register itself with the server; ownership passes to + * AsyncEventSource::_addClient(), which stores it in the shared client list * * @param client * @param server @@ -116,7 +132,7 @@ class AsyncEventSourceClient { return connected() && _queueMessage(std::move(message)); }; - [[deprecated("Use _write(AsyncEvent_SharedData_t message) instead to share same data with multiple SSE clients")]] + [[deprecated("Use write(AsyncEvent_SharedData_t message) instead to share same data with multiple SSE clients")]] bool write(const char *message, size_t len) { if (!connected()) { return false; @@ -185,12 +201,15 @@ class AsyncEventSourceClient { class AsyncEventSource : public AsyncWebHandler { private: String _url; - std::list> _clients; + std::list _clients; // Same as for individual messages, protect mutations of _clients list // since simultaneous access from different tasks is possible mutable asyncsrv::mutex_type _client_queue_lock; - ArEventHandlerFunction _connectcb = nullptr; - ArEventHandlerFunction _disconnectcb = nullptr; + // One callback slot per event: the raw-pointer setters (onConnect/onDisconnect) + // wrap their callback into the shared-handle form, so both flavors share storage + ArEventHandlerFunctionSafe _connectsafecb = nullptr; + ArEventHandlerFunctionSafe _disconnectsafecb = nullptr; + ArEventRequestConnectHandlerFunction _requestConnectcb = nullptr; // this method manipulates in-fligh data size for connected client depending on number of active connections void _adjust_inflight_window(); @@ -204,9 +223,9 @@ class AsyncEventSource : public AsyncWebHandler { AsyncEventSource(const char *url) : _url(url){}; AsyncEventSource(const String &url) : _url(url){}; - ~AsyncEventSource() { - close(); - }; + // Detaches clients still connected at destruction time so any app-held + // handles survive without touching the dead server (see AsyncEventSource.cpp) + ~AsyncEventSource(); const char *url() const { return _url.c_str(); @@ -215,13 +234,47 @@ class AsyncEventSource : public AsyncWebHandler { void close(); /** - * @brief set on-connect callback for the client + * @brief set on-connect callback for the client (raw-pointer flavor) * used to deliver messages to client on first connect + * @note the pointer is valid only during the callback and must not be stored. + * Shares the slot with onConnectSafe(): the last registered callback wins. + * @note prefer onConnectSafe(): it delivers a handle that may also be stored * * @param cb */ - void onConnect(ArEventHandlerFunction cb) { - _connectcb = cb; + void onConnect(ArEventHandlerFunction cb); + + /** + * @brief set on-connect callback delivering an ownership-safe shared handle + * The handle may be stored and used past the end of the callback (e.g. to + * close this connection later or to send targeted messages); the client + * object stays alive until the last handle is dropped. + */ + void onConnectSafe(ArEventHandlerFunctionSafe cb) { + _connectsafecb = cb; + } + + /** + * @brief set an on-connect callback that also receives the originating request + * @note the request pointer is valid only for the duration of the callback + * (read authentication/session state here); the client handle may be stored + */ + void onConnectWithRequest(ArEventRequestConnectHandlerFunction cb) { + _requestConnectcb = cb; + } + + // The client pointer sent to the callback is valid for the duration of the callback + // (the server keeps the object alive while the callback runs); it must not be + // stored. Prefer onDisconnectSafe(): it delivers a handle that may be stored. + // Shares the slot with onDisconnectSafe(): the last registered callback wins. + void onDisconnect(ArEventHandlerFunction cb); + + /** + * @brief set on-disconnect callback delivering an ownership-safe shared handle + * Safe to call methods on the handle inside the callback. + */ + void onDisconnectSafe(ArEventHandlerFunctionSafe cb) { + _disconnectsafecb = cb; } /** @@ -242,10 +295,6 @@ class AsyncEventSource : public AsyncWebHandler { return send(message.c_str(), event, id, reconnect); } - // The client pointer sent to the callback is only for reference purposes. DO NOT CALL ANY METHOD ON IT ! - void onDisconnect(ArEventHandlerFunction cb) { - _disconnectcb = cb; - } void authorizeConnect(ArAuthorizeConnectHandler cb); // returns number of connected clients @@ -254,8 +303,18 @@ class AsyncEventSource : public AsyncWebHandler { // returns average number of messages pending in all client's queues size_t avgPacketsWaiting() const; + /** + * @brief Get a snapshot of all registered clients as shared handles. + * The returned handles stay valid while held, even if a client disconnects + * mid-iteration (it merely reports connected() == false). Use for targeted + * sends or closes without retaining unsafe raw pointers. + */ + std::vector getClients() const; + // system callbacks (do not call from user code!) - void _addClient(AsyncEventSourceClient *client); + // registers a pre-allocated client; when a request is supplied, adopts its + // AsyncClient (ownership transfer) before registration + void _addClient(const AsyncEventSourceClientPtr &client, AsyncWebServerRequest *request = nullptr); void _handleDisconnect(AsyncEventSourceClient *client); bool canHandle(AsyncWebServerRequest *request) const final; void handleRequest(AsyncWebServerRequest *request) final;