fix presets.json corruption, part 2: mutex for presets file writing

prevents concurrent presets.json writes from doSaveState() and savePreset()
This commit is contained in:
Frank
2025-12-29 20:48:07 +01:00
parent 6247ee066d
commit 2a56f9b658
3 changed files with 27 additions and 4 deletions

View File

@@ -38,7 +38,14 @@ static void doSaveState() {
bool persist = (presetToSave < 251); bool persist = (presetToSave < 251);
const char *filename = getFileName(persist); const char *filename = getFileName(persist);
if (!requestJSONBufferLock(10)) return; // will set fileDoc if (!requestJSONBufferLock(10)) return; // will set fileDoc // async write
// WLEDMM Acquire file mutex before writing presets.json or tmp.json
if (esp32SemTake(presetFileMux, 2500) != pdTRUE) {
USER_PRINTLN(F("doSaveState(): preset file busy, cannot write"));
releaseJSONBufferLock();
return;
}
initPresetsFile(); // just in case if someone deleted presets.json using /edit initPresetsFile(); // just in case if someone deleted presets.json using /edit
JsonObject sObj = doc.to<JsonObject>(); JsonObject sObj = doc.to<JsonObject>();
@@ -82,6 +89,8 @@ static void doSaveState() {
writeObjectToFileUsingId(filename, presetToSave, fileDoc); writeObjectToFileUsingId(filename, presetToSave, fileDoc);
if (persist) presetsModifiedTime = toki.second(); //unix time if (persist) presetsModifiedTime = toki.second(); //unix time
esp32SemGive(presetFileMux); // Release file mutex
releaseJSONBufferLock(); releaseJSONBufferLock();
updateFSInfo(); updateFSInfo();
@@ -316,7 +325,14 @@ void savePreset(byte index, const char* pname, JsonObject sObj)
} else { } else {
// this is a playlist or API call // this is a playlist or API call
if (sObj[F("playlist")].isNull()) { if (sObj[F("playlist")].isNull()) {
// we will save API call immediately (often causes presets.json corruption) // we will save API call immediately (often causes presets.json corruption in the past)
// WLEDMM Acquire file mutex before writing presets.json, to prevent presets.json corruption
if (esp32SemTake(presetFileMux, 2500) != pdTRUE) {
USER_PRINTLN(F("doSaveState(): preset file busy, cannot write"));
return; // early exit, no change
}
presetToSave = 0; presetToSave = 0;
if (index > 250 || !fileDoc) return; // cannot save API calls to temporary preset (255) if (index > 250 || !fileDoc) return; // cannot save API calls to temporary preset (255)
sObj.remove("o"); sObj.remove("o");
@@ -325,8 +341,11 @@ void savePreset(byte index, const char* pname, JsonObject sObj)
sObj.remove(F("error")); sObj.remove(F("error"));
sObj.remove(F("psave")); sObj.remove(F("psave"));
if (sObj["n"].isNull()) sObj["n"] = saveName; if (sObj["n"].isNull()) sObj["n"] = saveName;
initPresetsFile(); // just in case if someone deleted presets.json using /edit initPresetsFile(); // just in case if someone deleted presets.json using /edit
writeObjectToFileUsingId(getFileName(index<255), index, fileDoc); writeObjectToFileUsingId(getFileName(index<255), index, fileDoc);
esp32SemGive(presetFileMux); // Release file mutex
presetsModifiedTime = toki.second(); //unix time presetsModifiedTime = toki.second(); //unix time
updateFSInfo(); updateFSInfo();
} else { } else {

View File

@@ -480,11 +480,14 @@ void WLED::setup()
busDrawMux = xSemaphoreCreateRecursiveMutex(); // WLEDMM prevent concurrent running of strip.show and strip.service busDrawMux = xSemaphoreCreateRecursiveMutex(); // WLEDMM prevent concurrent running of strip.show and strip.service
segmentMux = xSemaphoreCreateRecursiveMutex(); // WLEDMM prevent segment changes while effects are running segmentMux = xSemaphoreCreateRecursiveMutex(); // WLEDMM prevent segment changes while effects are running
jsonBufferLockMutex = xSemaphoreCreateRecursiveMutex(); // WLEDMM prevent concurrent JSON buffer writing jsonBufferLockMutex = xSemaphoreCreateRecursiveMutex(); // WLEDMM prevent concurrent JSON buffer writing
if ((busDrawMux == nullptr) || (segmentMux == nullptr) || (jsonBufferLockMutex == nullptr)) presetFileMux = xSemaphoreCreateRecursiveMutex(); // WLEDMM prevent concurrent presets.json file writing
if ((busDrawMux == nullptr) || (segmentMux == nullptr) || (jsonBufferLockMutex == nullptr) || (presetFileMux == nullptr)) {
USER_PRINTLN(F("setup error: xSemaphoreCreateRecursiveMutex failed.")); // should never happen. USER_PRINTLN(F("setup error: xSemaphoreCreateRecursiveMutex failed.")); // should never happen.
}
xSemaphoreGiveRecursive(busDrawMux); // init semaphores to initially allow drawing xSemaphoreGiveRecursive(busDrawMux); // init semaphores to initially allow drawing
xSemaphoreGiveRecursive(segmentMux); xSemaphoreGiveRecursive(segmentMux);
xSemaphoreGiveRecursive(jsonBufferLockMutex); xSemaphoreGiveRecursive(jsonBufferLockMutex);
xSemaphoreGiveRecursive(presetFileMux);
#endif #endif
#ifdef ARDUINO_ARCH_ESP32 #ifdef ARDUINO_ARCH_ESP32

View File

@@ -7,7 +7,7 @@
*/ */
// version code in format yymmddb (b = daily build) // version code in format yymmddb (b = daily build)
#define VERSION 2512291 #define VERSION 2512292
// WLEDMM - you can check for this define in usermods, to only enabled WLEDMM specific code in the "right" fork. Its not defined in AC WLED. // WLEDMM - you can check for this define in usermods, to only enabled WLEDMM specific code in the "right" fork. Its not defined in AC WLED.
#define _MoonModules_WLED_ #define _MoonModules_WLED_
@@ -771,6 +771,7 @@ WLED_GLOBAL volatile bool OTAisRunning _INIT(false); // WLEDMM temporaril
WLED_GLOBAL SemaphoreHandle_t busDrawMux _INIT(nullptr); WLED_GLOBAL SemaphoreHandle_t busDrawMux _INIT(nullptr);
WLED_GLOBAL SemaphoreHandle_t segmentMux _INIT(nullptr); WLED_GLOBAL SemaphoreHandle_t segmentMux _INIT(nullptr);
WLED_GLOBAL SemaphoreHandle_t jsonBufferLockMutex _INIT(nullptr); WLED_GLOBAL SemaphoreHandle_t jsonBufferLockMutex _INIT(nullptr);
WLED_GLOBAL SemaphoreHandle_t presetFileMux _INIT(nullptr); // Protects presets.json file writes
#define esp32SemTake(mux,timeout) xSemaphoreTakeRecursive(mux, pdMS_TO_TICKS(timeout)) // convenience macro that expands to xSemaphoreTakeRecursive - timeout is in milliseconds #define esp32SemTake(mux,timeout) xSemaphoreTakeRecursive(mux, pdMS_TO_TICKS(timeout)) // convenience macro that expands to xSemaphoreTakeRecursive - timeout is in milliseconds
#define esp32SemGive(mux) xSemaphoreGiveRecursive(mux) // convenience macro that expands to xSemaphoreGiveRecursive #define esp32SemGive(mux) xSemaphoreGiveRecursive(mux) // convenience macro that expands to xSemaphoreGiveRecursive
#define WLED_create_spinlock(theSname) static portMUX_TYPE theSname = portMUX_INITIALIZER_UNLOCKED #define WLED_create_spinlock(theSname) static portMUX_TYPE theSname = portMUX_INITIALIZER_UNLOCKED