From 6f01896d0443d3cb4029fc4b8c7167bd66d2c3b4 Mon Sep 17 00:00:00 2001 From: Will Miles Date: Sun, 4 Aug 2024 14:02:05 -0400 Subject: [PATCH] requestJSONBufferLock: Fix locking implementation On ESP8266, it isn't permissible to call delay() in system context; ensure this is legal before waiting. On ESP32, use an operating system mutex to ensure consistent variable state in a multicore environment, and manage the wait without needing to loop. --- wled00/util.cpp | 27 +++++++++++++++++++++++---- wled00/wled.h | 1 + 2 files changed, 24 insertions(+), 4 deletions(-) diff --git a/wled00/util.cpp b/wled00/util.cpp index 6bc02234b..d2302a085 100644 --- a/wled00/util.cpp +++ b/wled00/util.cpp @@ -213,15 +213,31 @@ bool requestJSONBufferLock(uint8_t module) DEBUG_PRINTLN(F("ERROR: JSON buffer not allocated!")); return false; } - unsigned long now = millis(); - - while (jsonBufferLock && millis()-now < 250) delay(1); // wait for fraction for buffer lock +#if defined(ARDUINO_ARCH_ESP32) + // Use a recursive mutex type in case our task is the one holding the JSON buffer. + // This can happen during large JSON web transactions. In this case, we continue immediately + // and then will return out below if the lock is still held. + if (xSemaphoreTakeRecursive(jsonBufferLockMutex, 250) == pdFALSE) return false; // timed out waiting +#elif defined(ARDUINO_ARCH_ESP8266) + // If we're in system context, delay() won't return control to the user context, so there's + // no point in waiting. + if (can_yield()) { + unsigned long now = millis(); + while (jsonBufferLock && (millis()-now < 250)) delay(1); // wait for fraction for buffer lock + } +#else + #error Unsupported task framework - fix requestJSONBufferLock +#endif + // If the lock is still held - by us, or by another task if (jsonBufferLock) { DEBUG_PRINT(F("ERROR: Locking JSON buffer failed! (still locked by ")); DEBUG_PRINT(jsonBufferLock); DEBUG_PRINTLN(")"); - return false; // waiting time-outed +#ifdef ARDUINO_ARCH_ESP32 + xSemaphoreGiveRecursive(jsonBufferLockMutex); +#endif + return false; } jsonBufferLock = module ? module : 255; @@ -239,6 +255,9 @@ void releaseJSONBufferLock() DEBUG_PRINT(jsonBufferLock); DEBUG_PRINTLN(")"); jsonBufferLock = 0; +#ifdef ARDUINO_ARCH_ESP32 + xSemaphoreGiveRecursive(jsonBufferLockMutex); +#endif } diff --git a/wled00/wled.h b/wled00/wled.h index f761b970d..d6915e7fb 100644 --- a/wled00/wled.h +++ b/wled00/wled.h @@ -810,6 +810,7 @@ WLED_GLOBAL int8_t spi_sclk _INIT(SPISCLKPIN); // global ArduinoJson buffer #if defined(ARDUINO_ARCH_ESP32) WLED_GLOBAL JsonDocument *pDoc _INIT(nullptr); +WLED_GLOBAL SemaphoreHandle_t jsonBufferLockMutex _INIT(xSemaphoreCreateRecursiveMutex()); #else WLED_GLOBAL StaticJsonDocument gDoc; WLED_GLOBAL JsonDocument *pDoc _INIT(&gDoc);