From 6382d2b7302b1318d3180fe47b7e84bb1b02a9b2 Mon Sep 17 00:00:00 2001 From: Will Miles Date: Sat, 6 Jan 2024 10:05:18 -0500 Subject: [PATCH 1/3] Add C++-style guard class for JSON Buffer lock Add JSONBufferGuard, an RAII lock guard class for JSONBufferLock analogous to std::lock_guard. This permits binding the guard to a scope, such as an object life cycle or function body, guaranteeing that the lock will be released when the scope exits. --- wled00/fcn_declare.h | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/wled00/fcn_declare.h b/wled00/fcn_declare.h index ec7527269..e256ceb5f 100644 --- a/wled00/fcn_declare.h +++ b/wled00/fcn_declare.h @@ -360,6 +360,22 @@ um_data_t* simulateSound(uint8_t simulationId); void enumerateLedmaps(); uint8_t get_random_wheel_index(uint8_t pos); +// RAII guard class for the JSON Buffer lock +// Modeled after std::lock_guard +class JSONBufferGuard { + bool holding_lock; + public: + inline JSONBufferGuard(uint8_t module=255) : holding_lock(requestJSONBufferLock(module)) {}; + inline ~JSONBufferGuard() { if (holding_lock) releaseJSONBufferLock(); }; + inline JSONBufferGuard(const JSONBufferGuard&) = delete; // Noncopyable + inline JSONBufferGuard& operator=(const JSONBufferGuard&) = delete; + inline JSONBufferGuard(JSONBufferGuard&& r) : holding_lock(r.holding_lock) { r.holding_lock = false; }; // but movable + inline JSONBufferGuard& operator=(JSONBufferGuard&& r) { holding_lock |= r.holding_lock; r.holding_lock = false; return *this; }; + inline bool owns_lock() const { return holding_lock; } + explicit inline operator bool() const { return owns_lock(); }; + inline void release() { if (holding_lock) releaseJSONBufferLock(); holding_lock = false; } +}; + #ifdef WLED_ADD_EEPROM_SUPPORT //wled_eeprom.cpp void applyMacro(byte index); From 9d3c0f4ff082d7f596c142e32e42eda607d6c1ae Mon Sep 17 00:00:00 2001 From: Will Miles Date: Sat, 6 Jan 2024 10:09:07 -0500 Subject: [PATCH 2/3] serveJson: Ensure semaphore scope lasts until reply is done Extend the JSON response class to hold the global JSON buffer lock until the transaction is completed. Fixes #3641 --- wled00/json.cpp | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/wled00/json.cpp b/wled00/json.cpp index ef2bdf346..d9bbb74ce 100644 --- a/wled00/json.cpp +++ b/wled00/json.cpp @@ -1009,6 +1009,17 @@ void serializeModeNames(JsonArray arr) } } + +// Global buffer locking response helper class +class GlobalBufferAsyncJsonResponse: public JSONBufferGuard, public AsyncJsonResponse { + public: + inline GlobalBufferAsyncJsonResponse(bool isArray) : JSONBufferGuard(17), AsyncJsonResponse(pDoc, isArray) {}; + virtual ~GlobalBufferAsyncJsonResponse() {}; + + // Other members are inherited +}; + + static volatile bool servingClient = false; void serveJson(AsyncWebServerRequest* request) { @@ -1050,12 +1061,12 @@ void serveJson(AsyncWebServerRequest* request) return; } - if (!requestJSONBufferLock(17)) { + GlobalBufferAsyncJsonResponse *response = new GlobalBufferAsyncJsonResponse(subJson==JSON_PATH_FXDATA || subJson==JSON_PATH_EFFECTS); // will clear and convert JsonDocument into JsonArray if necessary + if (!response->owns_lock()) { serveJsonError(request, 503, ERR_NOBUF); servingClient = false; return; } - AsyncJsonResponse *response = new AsyncJsonResponse(pDoc, subJson==JSON_PATH_FXDATA || subJson==JSON_PATH_EFFECTS); // will clear and convert JsonDocument into JsonArray if necessary JsonVariant lDoc = response->getRoot(); @@ -1098,7 +1109,6 @@ void serveJson(AsyncWebServerRequest* request) DEBUG_PRINT(F("JSON content length: ")); DEBUG_PRINTLN(len); request->send(response); - releaseJSONBufferLock(); servingClient = false; } From 77116172e466a0c33f9f460e0ff56a838e5c0ec4 Mon Sep 17 00:00:00 2001 From: Will Miles Date: Sat, 6 Jan 2024 10:24:05 -0500 Subject: [PATCH 3/3] serveJson: Fix possible memory leak Ensure we delete the response if it's not locked --- wled00/json.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/wled00/json.cpp b/wled00/json.cpp index d9bbb74ce..ccec74ed8 100644 --- a/wled00/json.cpp +++ b/wled00/json.cpp @@ -1065,6 +1065,7 @@ void serveJson(AsyncWebServerRequest* request) if (!response->owns_lock()) { serveJsonError(request, 503, ERR_NOBUF); servingClient = false; + delete response; return; }