Repository navigation
Conversation
mathieucarbou
left a comment
There was a problem hiding this comment.
I will let @me-no-dev and @willmmiles also review.
I am not against the idea of allowing an easy usage of PSRAM for the library especially for objects that are a long lived. This would probably be a good and easy way to free some memory easily for users running with PSRAM.
But I am also wondering why you picked only this place ? For example, middleware could benefit of that too, and a few others little classes also. If we accept the idea, I would probably like to apply this pattern more globally across the project where it can be applied.
| if (!p) { | ||
| p = malloc(size); | ||
| } | ||
| return p; |
There was a problem hiding this comment.
I think that if new fails on ESP32 it does throw std::bad_alloc (which crashes the esp). Here it would return null right ?
There was a problem hiding this comment.
Correct, operator new(size_t) is required to throw std::bad_alloc on an allocation failure. operator new(size_t, const std::nothrow&) is the variant that returns null.
^^^ This. Why just the handler objects? Why not the request objects? The middleware? Response objects? The buffers used by the response objects (usually much bigger than the requests, and probably the largest allocations in the normal path)? The parsed header list entries? ..and so on. I'm not opposed to considering support for custom allocators, but I don't think it makes sense to apply it ad-hoc on only one object type/family. I'd like to see an interface that's scoped over the whole library, with careful selection of which objects and where it's applied and some discussion as to which objects and data structures were selected or not selected, and why. As a second concern, any alternative allocation API should delegate the implementation details to the library user, rather than binding a single specific allocation flavour. Supporting only |
|
Fair points on both counts, thanks! On the operator new semantics: agreed... this should either be the std::nothrow overload pair, or the throwing variant with a proper std::bad_alloc. In our production firmware the null case only occurs when both heaps are exhausted (the device is beyond recovery at that point anyway), but that's no excuse for non-conforming semantics in a library. On scope: I deliberately kept the PR minimal to gauge interest, but I fully support going broader. A pluggable allocator interface (as @willmmiles suggests, similar to what ArduinoJson does) seems like the most future-proof design. I'm happy to rework the PR in that direction if you can settle the preferred shape (static allocator hooks vs. an allocator object), or equally happy if you'd rather drive the design yourselves. |
I like the interface idea, especially because we do not have to deal with the many specific user implémentations. But how to decide when the allocator would be used ? Using PSRAM is slower and psram is best suited for large buffers like images. So one may want to not use psram for requests and responses because he has an app with a lot of short lived clients. also, frequently accessed memory would be in the shared cache with ram: so how much do we gain in putting middleware’s for example, where global ones like security would be accessed on every requests ? So even scoping the psram usage is complicated. in the case of middlewares that is easy because they are created by the user (except if a function is passed). So there is already a way to have the user control how they are allocated. but there could be some situations where this is more fuzzy. For example if someone setups a route for /* as a fallback route, or an error handler , this one probably should be on heap and not psram for speed concerns ? So maybe we could first look at the scopes : what could be and cannot be in psram, and also what kind of allocations the use control. for example, if the user had a way to add handlers to the server in the same way it is done for middleware, then the user could control itself allocation. so maybe we first need also to look if there is a way to refactor the lib to allow the user control a bit more the allocation part per object ? |
|
Let me share some field data on the speed concern first. On the ESP32-S3, PSRAM access goes through the data cache, so small objects that are touched frequently, like a middleware struct or a handler with a callback, are effectively cache resident. After moving all handler objects to PSRAM in our production firmware we could not measure any per request difference, and that firmware has been running since mid July with a dashboard heavy workload and multiple websocket clients. The real limitations of PSRAM are interrupt and DMA access and the short windows during flash writes where the cache is disabled. Neither of those applies to request dispatch in the async_tcp task. For us the motivation was never speed anyway. When internal SRAM runs out, the server does not get slow, it takes down WiFi, mbedTLS and ESP-NOW allocations with it. Compared to keeping the radio alive, a theoretical few microseconds per request is not a trade off in practice. I think your middleware point actually draws the scope line quite nicely. Everything the user constructs themselves, such as middlewares, an AsyncWebSocket instance or custom handlers, is already under user control today. The user can allocate those however they want, so at most this needs documentation. The gap is in the long lived allocations that happen inside the library, where the user has no influence at all: the AsyncCallbackWebHandler created inside server.on(), the AsyncStaticWebHandler from serveStatic(), rewrites and the catch all handler. That is exactly why my PR targeted the handler base class, because that is where all the uncontrollable allocations funnel through. So maybe we can avoid the whole per object policy debate with a phased approach. As a first step, the library gets one global allocator hook, for example two static function pointers or a minimal interface, with the default being the current malloc and new behavior. This hook would only be applied to the long lived objects created by the library itself. The library makes no policy decisions at all. Whoever installs the allocator decides what happens, and by default nothing changes for anyone. Short lived per request objects would stay untouched for now and could be evaluated separately later, since that is where your concern about apps with many short lived clients is entirely valid. I am happy to rework this PR into that first step if the shape works for you, and equally happy to leave the design to you. For our use case, any opt in mechanism that gets the library created long lived objects out of internal SRAM solves the problem. |
Yes I agree that this can be a good starting point. But from an API design perspective it feels wrong to me: it is like providing a patch to the user for something that could be refactored nicely. for example for the on() calls to setup routes: why won’t we allow these methods to take a user constructed handler as parameter directly ? this would also allow the user to have its route in static ram if they want to (flash) and they could even associate the same handler object to several different routes, like: async callback… c = … Such refactoring would let the user have a complete control over allocation but also whether the handers are constructed on heap or static ram on flash. Also, reusing handlers would be possible. I see more advantage of a refactoring of the library than introducing an allocator interface. what do you think guys ? |
|
I like that direction, honestly more than my own proposal. If the user can construct and pass in the handler, allocation control comes for free and no allocator machinery is needed at all. From an API perspective that is the cleaner end state. Two design questions probably need an answer early, because they decide how deep the refactoring goes. The first is ownership. Today the server owns and deletes the handlers it creates internally. Once users pass in their own objects, and especially once the same handler can be attached to several routes as in your example, the server can no longer simply delete what it holds, otherwise we get double deletes. So registration would need to be non owning, or based on shared_ptr, with a clear rule for who destroys what. The second is that the URI currently lives inside the handler itself. AsyncCallbackWebHandler carries its own pattern and method mask, so attaching one handler object to several routes only becomes possible once the route matching moves out of the handler and into the server's routing table. That is the actual refactoring work hiding behind the nice two line example. One small note on flash: handlers are mutable objects, they carry filters, the middleware chain and in the websocket case the client list, so placing them truly in flash would require a const redesign. Static RAM placement on the other hand would just work once construction is in user hands. The convenience overload of on() with a lambda should probably survive in any case, since that is what nearly every existing sketch uses. It would keep allocating internally, which seems fine to me as long as the handler object path exists for those who care about where things live. For what it is worth, we are happy with this as the solution and in no hurry, since we currently carry a small local patch that keeps our firmware running. |
Yes this would be a larger refactoring indeed. |
|
This PR is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 7 days. |
|
Friendly reminder |
Decouple URI/method matching from handler objects by introducing a
unified handler list where each entry optionally carries route matching
info (AsyncURIMatcher + WebRequestMethodComposite). This enables one
handler object to serve multiple routes, combining Phase 1's ownership
control with Phase 2's routing flexibility.
Changes:
- HandlerEntry struct unified: { handler, owned, hasMatcher, uri*, method }
- hasMatcher=true → server does URI+method matching (route entry)
- hasMatcher=false → legacy canHandle()-based dispatch
- Single std::list<HandlerEntry> _handlers (no more _routes/_handlers split)
- addRoute(uri, method, handlerPtr, takeOwnership=true) — owning registration
- addRoute(uri, method, handlerRef) — non-owning registration
- removeRoute(uri, method) — remove a specific route entry
- removeHandler() works uniformly for both route and legacy entries
- on() convenience wrappers now create handlers and register via addRoute()
- serveStatic() unchanged (still uses addHandler — static handler has own URI logic)
- _attachHandler() unified dispatch: iterate single list, routes first by
hasMatcher flag (insertion order preserved across both kinds)
- AsyncCallbackWebHandler::_routeManaged flag — when set, canHandle()
skips URI/method checks (server's route table does them)
- AsyncCallbackJsonWebHandler::_routeManaged — same support for JSON handlers
- _setRouteManaged() is protected virtual on AsyncWebHandler (no-op default),
friend class AsyncWebServer — internal API hidden from users (RTTI-free,
no dynamic_cast, works with -fno-rtti on all platforms)
- AsyncURIMatcher::operator==/!= for route removal by value comparison
- reset() clears the unified list, properly deleting uri matchers
- New MultiRoute example: one handler on 3 routes (static RAM),
PSRAM handler on 2 routes, classic on() handler, custom catch-all
- Updated docs: routing.md (Route Table section), configuration.md,
concepts.md
Backward compatible: all Phase 1 APIs and existing on()/addHandler()/
serveStatic() calls work identically. Routes are additive.
Builds on Phase 1 (non-owning handler registration).
Motivation from #468 discussion.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Give users full control over where handler objects live in memory (internal SRAM, PSRAM, static RAM) without allocator hooks or operator new overrides. Changes: - asyncsrv::MaybeOwned<T>: tagged pointer tracking ownership in a single machine word (1 pointer instead of 2 — halves per-entry SRAM usage). Low bit = ownership flag. adopt() for owning, borrow() for non-owning. - std::list<asyncsrv::MaybeOwned<AsyncWebHandler>> _handlers — unified list replacing the old unique_ptr list. - asyncsrv::MaybeOwned<AsyncCallbackWebHandler> _catchAllHandler — catch-all uses the same ownership mechanism (no separate bool flag). - addHandler(ptr) — server takes ownership (backward compatible) - addHandler(ref) — non-owning borrow (reference = borrow in C++) - AsyncWebServer(port, catchAllPtr) — custom server-owned catch-all - AsyncWebServer(port, catchAllRef) — custom non-owning catch-all - setCatchAllHandler(ptr) — replace with server-owned catch-all - setCatchAllHandler(ref) — replace with non-owning catch-all - Catch-all invariant preserved: always installed, never nullptr (assert). catchAllHandler() restored without deprecation — always safe to call. - removeHandler(ptr) — removes from unified list, deletes if owned - New CustomAllocation example: static RAM, PSRAM, server-owned patterns - Updated docs: routing.md, configuration.md, concepts.md Fully backward compatible: all existing APIs work unchanged. - server.addHandler(new T()) — unchanged behavior - server.addHandler(&staticObj) — unchanged behavior - server.on() / serveStatic() / catchAllHandler() / onNotFound() — unchanged - No existing examples need modification (SkipServerMiddleware untouched) Motivation: ESP32 internal SRAM is scarce (lwIP, WiFi buffers). Long-lived handler objects can now be moved to PSRAM or static RAM, freeing internal SRAM for networking. See discussion in #468. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Give users full control over where handler objects live in memory (internal SRAM, PSRAM, static RAM) without allocator hooks or operator new overrides. Changes: - asyncsrv::MaybeOwned<T>: tagged pointer tracking ownership in a single machine word (1 pointer instead of 2 — halves per-entry SRAM usage). Low bit = ownership flag. adopt() for owning, borrow() for non-owning. - std::list<asyncsrv::MaybeOwned<AsyncWebHandler>> _handlers — unified list replacing the old unique_ptr list. - asyncsrv::MaybeOwned<AsyncCallbackWebHandler> _catchAllHandler — catch-all uses the same ownership mechanism (no separate bool flag). - addHandler(ptr) — server takes ownership (backward compatible) - addHandler(ref) — non-owning borrow (reference = borrow in C++) - AsyncWebServer(port, catchAllPtr) — custom server-owned catch-all - AsyncWebServer(port, catchAllRef) — custom non-owning catch-all - setCatchAllHandler(ptr) — replace with server-owned catch-all - setCatchAllHandler(ref) — replace with non-owning catch-all - Catch-all invariant preserved: always installed, never nullptr (assert). catchAllHandler() restored without deprecation — always safe to call. - removeHandler(ptr) — removes from unified list, deletes if owned - New CustomAllocation example: static RAM, PSRAM, server-owned patterns - Updated docs: routing.md, configuration.md, concepts.md Fully backward compatible: all existing APIs work unchanged. - server.addHandler(new T()) — unchanged behavior - server.addHandler(&staticObj) — unchanged behavior - server.on() / serveStatic() / catchAllHandler() / onNotFound() — unchanged - No existing examples need modification (SkipServerMiddleware untouched) Motivation: ESP32 internal SRAM is scarce (lwIP, WiFi buffers). Long-lived handler objects can now be moved to PSRAM or static RAM, freeing internal SRAM for networking. See discussion in #468. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Give users full control over where handler objects live in memory (internal SRAM, PSRAM, static RAM) without allocator hooks or operator new overrides. Changes: - asyncsrv::MaybeOwned<T>: tagged pointer tracking ownership in a single machine word (1 pointer instead of 2 — halves per-entry SRAM usage). Low bit = ownership flag. adopt() for owning, borrow() for non-owning. - std::list<asyncsrv::MaybeOwned<AsyncWebHandler>> _handlers — unified list replacing the old unique_ptr list. - asyncsrv::MaybeOwned<AsyncCallbackWebHandler> _catchAllHandler — catch-all uses the same ownership mechanism (no separate bool flag). - addHandler(ptr) — server takes ownership (backward compatible) - addHandler(ref) — non-owning borrow (reference = borrow in C++) - AsyncWebServer(port, catchAllPtr) — custom server-owned catch-all - AsyncWebServer(port, catchAllRef) — custom non-owning catch-all - setCatchAllHandler(ptr) — replace with server-owned catch-all - setCatchAllHandler(ref) — replace with non-owning catch-all - Catch-all invariant preserved: always installed, never nullptr (assert). catchAllHandler() restored without deprecation — always safe to call. - removeHandler(ptr) — removes from unified list, deletes if owned - New CustomAllocation example: static RAM, PSRAM, server-owned patterns - Updated docs: routing.md, configuration.md, concepts.md Fully backward compatible: all existing APIs work unchanged. - server.addHandler(new T()) — unchanged behavior - server.addHandler(&staticObj) — unchanged behavior - server.on() / serveStatic() / catchAllHandler() / onNotFound() — unchanged - No existing examples need modification (SkipServerMiddleware untouched) Motivation: ESP32 internal SRAM is scarce (lwIP, WiFi buffers). Long-lived handler objects can now be moved to PSRAM or static RAM, freeing internal SRAM for networking. See discussion in #468. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Give users full control over where handler objects live in memory (internal SRAM, PSRAM, static RAM) without allocator hooks or operator new overrides. Changes: - asyncsrv::MaybeOwned<T>: tagged pointer tracking ownership in a single machine word (1 pointer instead of 2 — halves per-entry SRAM usage). Low bit = ownership flag. adopt() for owning, borrow() for non-owning. - std::list<asyncsrv::MaybeOwned<AsyncWebHandler>> _handlers — unified list replacing the old unique_ptr list. - asyncsrv::MaybeOwned<AsyncCallbackWebHandler> _catchAllHandler — catch-all uses the same ownership mechanism (no separate bool flag). - addHandler(ptr) — server takes ownership (backward compatible) - addHandler(ref) — non-owning borrow (reference = borrow in C++) - AsyncWebServer(port, catchAllPtr) — custom server-owned catch-all - AsyncWebServer(port, catchAllRef) — custom non-owning catch-all - setCatchAllHandler(ptr) — replace with server-owned catch-all - setCatchAllHandler(ref) — replace with non-owning catch-all - Catch-all invariant preserved: always installed, never nullptr (assert). catchAllHandler() restored without deprecation — always safe to call. - removeHandler(ptr) — removes from unified list, deletes if owned - New CustomAllocation example: static RAM, PSRAM, server-owned patterns - Updated docs: routing.md, configuration.md, concepts.md Fully backward compatible: all existing APIs work unchanged. - server.addHandler(new T()) — unchanged behavior - server.addHandler(&staticObj) — unchanged behavior - server.on() / serveStatic() / catchAllHandler() / onNotFound() — unchanged - No existing examples need modification (SkipServerMiddleware untouched) Motivation: ESP32 internal SRAM is scarce (lwIP, WiFi buffers). Long-lived handler objects can now be moved to PSRAM or static RAM, freeing internal SRAM for networking. See discussion in #468. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
) * feat: user-controlled handler allocation (non-owning registration) Give users full control over where handler objects live in memory (internal SRAM, PSRAM, static RAM) without allocator hooks or operator new overrides. Changes: - asyncsrv::MaybeOwned<T>: tagged pointer tracking ownership in a single machine word (1 pointer instead of 2 — halves per-entry SRAM usage). Low bit = ownership flag. adopt() for owning, borrow() for non-owning. - std::list<asyncsrv::MaybeOwned<AsyncWebHandler>> _handlers — unified list replacing the old unique_ptr list. - asyncsrv::MaybeOwned<AsyncCallbackWebHandler> _catchAllHandler — catch-all uses the same ownership mechanism (no separate bool flag). - addHandler(ptr) — server takes ownership (backward compatible) - addHandler(ref) — non-owning borrow (reference = borrow in C++) - AsyncWebServer(port, catchAllPtr) — custom server-owned catch-all - AsyncWebServer(port, catchAllRef) — custom non-owning catch-all - setCatchAllHandler(ptr) — replace with server-owned catch-all - setCatchAllHandler(ref) — replace with non-owning catch-all - Catch-all invariant preserved: always installed, never nullptr (assert). catchAllHandler() restored without deprecation — always safe to call. - removeHandler(ptr) — removes from unified list, deletes if owned - New CustomAllocation example: static RAM, PSRAM, server-owned patterns - Updated docs: routing.md, configuration.md, concepts.md Fully backward compatible: all existing APIs work unchanged. - server.addHandler(new T()) — unchanged behavior - server.addHandler(&staticObj) — unchanged behavior - server.on() / serveStatic() / catchAllHandler() / onNotFound() — unchanged - No existing examples need modification (SkipServerMiddleware untouched) Motivation: ESP32 internal SRAM is scarce (lwIP, WiFi buffers). Long-lived handler objects can now be moved to PSRAM or static RAM, freeing internal SRAM for networking. See discussion in #468. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * ci(pre-commit): Apply automatic fixes --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: pre-commit-ci-lite[bot] <117423508+pre-commit-ci-lite[bot]@users.noreply.github.com>
A handler can now serve multiple URIs by holding a list of matchers instead of a single one: - setUri() replaces the list with one matcher (backward-compatible) - addUri() appends an additional URI matcher - clearUris() removes all matchers One HTTP method mask (setMethod) applies to all URIs of a handler. Use separate handlers when different URIs need different methods (e.g. a GET handler and a POST handler with onBody/onUpload callbacks). Also in this change: - addHandler(ptr, takeOwnership) — non-owning registration via pointer (in addition to the existing reference overload) - Catch-all handler can be nullptr (opt-out): unmatched requests get a bare 501 response. onNotFound/onFileUpload/onRequestBody are no-ops. - catchAllHandlerPtr() — safe accessor returning nullptr when no catch-all - catchAllHandler() is deprecated in favor of catchAllHandlerPtr() - setCatchAllHandler(ptr, takeOwnership) and setCatchAllHandler(nullptr) - Updated SkipServerMiddleware example to use catchAllHandlerPtr() - Updated CustomAllocation example for the new registration API - New MultiRoute example: one handler serving multiple URIs in static RAM, PSRAM handler on multiple URIs, separate handlers per method - Updated docs: routing.md (Multiple URIs per Handler section), configuration.md, concepts.md Fully backward compatible: existing code using on(), addHandler(), serveStatic(), and single-URI handlers works identically. Motivation from #468 discussion. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
A handler can now serve multiple URIs by holding a list of matchers instead of a single one: - setUri() replaces the list with one matcher (backward-compatible) - addUri() appends an additional URI matcher - clearUris() removes all matchers One HTTP method mask (setMethod) applies to all URIs of a handler. Use separate handlers when different URIs need different methods (e.g. a GET handler and a POST handler with onBody/onUpload callbacks). Applies to both AsyncCallbackWebHandler and AsyncCallbackJsonWebHandler. - New MultiRoute example: one handler serving multiple URIs in static RAM, PSRAM handler on multiple URIs, separate handlers per method - Updated docs: routing.md (Multiple URIs per Handler section), configuration.md, concepts.md Fully backward compatible: existing code using on(), addHandler(), serveStatic(), and single-URI handlers works identically. Motivation from #468 discussion. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
A handler can now serve multiple URIs and accept multiple HTTP methods by holding a list of matchers instead of a single one: - setUri() replaces the list with one matcher (backward-compatible) - addUri() appends an additional URI matcher - clearUris() removes all matchers setMethod() accepts a composite mask (HTTP_GET | HTTP_POST) that applies to all URIs of a handler. Use separate handlers when different methods need different callbacks (onBody/onUpload). Applies to both AsyncCallbackWebHandler and AsyncCallbackJsonWebHandler. - New MultiRoute example: multiple URIs x composite methods, separate handlers for callbacks - Updated docs: routing.md (Multiple URIs per Handler section), configuration.md, concepts.md Fully backward compatible: existing code using on(), addHandler(), serveStatic(), and single-URI handlers works identically. Motivation from #468 discussion. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
A handler can now serve multiple URIs and accept multiple HTTP methods by holding a list of matchers instead of a single one: - setUri() replaces the list with one matcher (backward-compatible) - addUri() appends an additional URI matcher - clearUris() removes all matchers setMethod() accepts a composite mask (HTTP_GET | HTTP_POST) that applies to all URIs of a handler. Use separate handlers when different methods need different callbacks (onBody/onUpload). Applies to both AsyncCallbackWebHandler and AsyncCallbackJsonWebHandler. - New MultiRoute example: multiple URIs x composite methods, separate handlers for callbacks - Updated docs: routing.md (Multiple URIs per Handler section), configuration.md, concepts.md Fully backward compatible: existing code using on(), addHandler(), serveStatic(), and single-URI handlers works identically. Motivation from #468 discussion. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
On ESP32 targets with PSRAM, internal SRAM is often the scarcest resource (lwIP segments, WiFi buffers, WS message queues all live there). Every AsyncWebHandler-derived object (route handlers, catch-all, AsyncWebSocket, static file handlers) currently comes from the internal heap, though handlers are created once after PSRAM init and only read during request dispatch — never from ISR/DMA context, and latency is irrelevant for them.
This PR adds an opt-in build flag -D ASYNCWEBSERVER_USE_PSRAM that routes handler allocation through heap_caps_malloc(MALLOC_CAP_SPIRAM) with a transparent fallback to the internal heap, so it is safe even on targets without PSRAM. Without the flag, behavior is bit-identical to today — no risk for existing users.
Context: we run a dashboard-heavy ESP32-S3 firmware (same field setup that produced #453) where internal SRAM exhaustion cascades into WiFi/ESP-NOW allocation failures. Moving handlers out of internal RAM is one of the mitigations we have been running in production since mid-July.