From e0e5377da7dc1de4b1b7c5fb3dbd815a9e752731 Mon Sep 17 00:00:00 2001 From: Maxim Prokhorov Date: Sun, 6 Dec 2020 00:33:40 +0300 Subject: [PATCH] terminal: small workaround for _tempObject from API Exchange sneaky memory leak with a squeaky wheel... Web server will immediatly delete the request object, so not really much point in tracking the pointer via some external means. --- code/espurna/api.cpp | 31 --------------------------- code/espurna/api_impl.h | 31 +++++++++++++++++++++++++++ code/espurna/web_asyncwebprint_impl.h | 16 +++++++++++--- 3 files changed, 44 insertions(+), 34 deletions(-) diff --git a/code/espurna/api.cpp b/code/espurna/api.cpp index a07ea560..c460727e 100644 --- a/code/espurna/api.cpp +++ b/code/espurna/api.cpp @@ -268,37 +268,6 @@ bool _apiIsFormDataContent(AsyncWebServerRequest* request) { return _apiMatchHeader(request, F("Content-Type"), F("application/x-www-form-urlencoded")); } -struct ApiRequestHelper { - ApiRequestHelper(const ApiRequestHelper&) = delete; - ApiRequestHelper(ApiRequestHelper&&) noexcept = default; - - // &path is expected to be request->url(), which is valid throughout the request's lifetime - explicit ApiRequestHelper(AsyncWebServerRequest& request, const PathParts& pattern) : - _request(request), - _pattern(pattern), - _path(request.url()), - _match(_pattern.match(_path)) - {} - - ApiRequest request() const { - return ApiRequest(_request, _pattern, _path); - } - - const PathParts& parts() const { - return _path; - } - - bool match() const { - return _match; - } - -private: - AsyncWebServerRequest& _request; - const PathParts& _pattern; - PathParts _path; - bool _match; -}; - // Because the webserver request is split between multiple separate function invocations, we need to preserve some state. // TODO: in case we are dealing with multicore, perhaps enforcing static-size data structs instead of the vector would we better, // to avoid calling generic malloc when paths are parsed? diff --git a/code/espurna/api_impl.h b/code/espurna/api_impl.h index d0a2de08..b1444489 100644 --- a/code/espurna/api_impl.h +++ b/code/espurna/api_impl.h @@ -180,3 +180,34 @@ private: const PathParts& _pattern; const PathParts& _parts; }; + +struct ApiRequestHelper { + ApiRequestHelper(const ApiRequestHelper&) = delete; + ApiRequestHelper(ApiRequestHelper&&) noexcept = default; + + // &path is expected to be request->url(), which is valid throughout the request's lifetime + explicit ApiRequestHelper(AsyncWebServerRequest& request, const PathParts& pattern) : + _request(request), + _pattern(pattern), + _path(request.url()), + _match(_pattern.match(_path)) + {} + + ApiRequest request() const { + return ApiRequest(_request, _pattern, _path); + } + + const PathParts& parts() const { + return _path; + } + + bool match() const { + return _match; + } + +private: + AsyncWebServerRequest& _request; + const PathParts& _pattern; + PathParts _path; + bool _match; +}; diff --git a/code/espurna/web_asyncwebprint_impl.h b/code/espurna/web_asyncwebprint_impl.h index c4967768..6c529df3 100644 --- a/code/espurna/web_asyncwebprint_impl.h +++ b/code/espurna/web_asyncwebprint_impl.h @@ -15,7 +15,8 @@ Copyright (C) 2016-2019 by Xose Pérez #if WEB_SUPPORT -namespace asyncwebprint_traits { +namespace asyncwebprint { +namespace traits { template using print_callable_t = decltype(std::declval()(std::declval())); @@ -23,17 +24,26 @@ using print_callable_t = decltype(std::declval()(std::declval())); template using is_print_callable = is_detected; +} } template void AsyncWebPrint::scheduleFromRequest(const AsyncWebPrintConfig& config, AsyncWebServerRequest* request, CallbackType callback) { - static_assert(asyncwebprint_traits::is_print_callable::value, "CallbackType needs to be a callable with void(Print&)"); + static_assert(asyncwebprint::traits::is_print_callable::value, "CallbackType needs to be a callable with void(Print&)"); // because of async nature of the server, we need to make sure we outlive 'request' object auto print = std::shared_ptr(new AsyncWebPrint(config, request)); // attach one ptr to onDisconnect capture, so we can detect disconnection before scheduled function runs - request->onDisconnect([print]() { + request->onDisconnect([print, request]() { +#if API_SUPPORT + // TODO: in case this comes from `apiRegister`'ed endpoint, there's still a lingering ApiRequestHelper that we must remove + if (request->_tempObject) { + auto* ptr = reinterpret_cast(request->_tempObject); + delete ptr; + request->_tempObject = nullptr; + } +#endif print->setState(AsyncWebPrint::State::Done); });