Skip to content

Commit 2a56f9b

Browse files
committed
fix presets.json corruption, part 2: mutex for presets file writing
prevents concurrent presets.json writes from doSaveState() and savePreset()
1 parent 6247ee0 commit 2a56f9b

File tree

3 files changed

+27
-4
lines changed

3 files changed

+27
-4
lines changed

wled00/presets.cpp

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,14 @@ static void doSaveState() {
3838
bool persist = (presetToSave < 251);
3939
const char *filename = getFileName(persist);
4040

41-
if (!requestJSONBufferLock(10)) return; // will set fileDoc
41+
if (!requestJSONBufferLock(10)) return; // will set fileDoc // async write
42+
43+
// WLEDMM Acquire file mutex before writing presets.json or tmp.json
44+
if (esp32SemTake(presetFileMux, 2500) != pdTRUE) {
45+
USER_PRINTLN(F("doSaveState(): preset file busy, cannot write"));
46+
releaseJSONBufferLock();
47+
return;
48+
}
4249

4350
initPresetsFile(); // just in case if someone deleted presets.json using /edit
4451
JsonObject sObj = doc.to<JsonObject>();
@@ -82,6 +89,8 @@ static void doSaveState() {
8289
writeObjectToFileUsingId(filename, presetToSave, fileDoc);
8390

8491
if (persist) presetsModifiedTime = toki.second(); //unix time
92+
93+
esp32SemGive(presetFileMux); // Release file mutex
8594
releaseJSONBufferLock();
8695
updateFSInfo();
8796

@@ -316,7 +325,14 @@ void savePreset(byte index, const char* pname, JsonObject sObj)
316325
} else {
317326
// this is a playlist or API call
318327
if (sObj[F("playlist")].isNull()) {
319-
// we will save API call immediately (often causes presets.json corruption)
328+
// we will save API call immediately (often causes presets.json corruption in the past)
329+
330+
// WLEDMM Acquire file mutex before writing presets.json, to prevent presets.json corruption
331+
if (esp32SemTake(presetFileMux, 2500) != pdTRUE) {
332+
USER_PRINTLN(F("doSaveState(): preset file busy, cannot write"));
333+
return; // early exit, no change
334+
}
335+
320336
presetToSave = 0;
321337
if (index > 250 || !fileDoc) return; // cannot save API calls to temporary preset (255)
322338
sObj.remove("o");
@@ -325,8 +341,11 @@ void savePreset(byte index, const char* pname, JsonObject sObj)
325341
sObj.remove(F("error"));
326342
sObj.remove(F("psave"));
327343
if (sObj["n"].isNull()) sObj["n"] = saveName;
344+
328345
initPresetsFile(); // just in case if someone deleted presets.json using /edit
329346
writeObjectToFileUsingId(getFileName(index<255), index, fileDoc);
347+
348+
esp32SemGive(presetFileMux); // Release file mutex
330349
presetsModifiedTime = toki.second(); //unix time
331350
updateFSInfo();
332351
} else {

wled00/wled.cpp

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -480,11 +480,14 @@ void WLED::setup()
480480
busDrawMux = xSemaphoreCreateRecursiveMutex(); // WLEDMM prevent concurrent running of strip.show and strip.service
481481
segmentMux = xSemaphoreCreateRecursiveMutex(); // WLEDMM prevent segment changes while effects are running
482482
jsonBufferLockMutex = xSemaphoreCreateRecursiveMutex(); // WLEDMM prevent concurrent JSON buffer writing
483-
if ((busDrawMux == nullptr) || (segmentMux == nullptr) || (jsonBufferLockMutex == nullptr))
483+
presetFileMux = xSemaphoreCreateRecursiveMutex(); // WLEDMM prevent concurrent presets.json file writing
484+
if ((busDrawMux == nullptr) || (segmentMux == nullptr) || (jsonBufferLockMutex == nullptr) || (presetFileMux == nullptr)) {
484485
USER_PRINTLN(F("setup error: xSemaphoreCreateRecursiveMutex failed.")); // should never happen.
486+
}
485487
xSemaphoreGiveRecursive(busDrawMux); // init semaphores to initially allow drawing
486488
xSemaphoreGiveRecursive(segmentMux);
487489
xSemaphoreGiveRecursive(jsonBufferLockMutex);
490+
xSemaphoreGiveRecursive(presetFileMux);
488491
#endif
489492

490493
#ifdef ARDUINO_ARCH_ESP32

wled00/wled.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
*/
88

99
// version code in format yymmddb (b = daily build)
10-
#define VERSION 2512291
10+
#define VERSION 2512292
1111

1212
// 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.
1313
#define _MoonModules_WLED_
@@ -771,6 +771,7 @@ WLED_GLOBAL volatile bool OTAisRunning _INIT(false); // WLEDMM temporaril
771771
WLED_GLOBAL SemaphoreHandle_t busDrawMux _INIT(nullptr);
772772
WLED_GLOBAL SemaphoreHandle_t segmentMux _INIT(nullptr);
773773
WLED_GLOBAL SemaphoreHandle_t jsonBufferLockMutex _INIT(nullptr);
774+
WLED_GLOBAL SemaphoreHandle_t presetFileMux _INIT(nullptr); // Protects presets.json file writes
774775
#define esp32SemTake(mux,timeout) xSemaphoreTakeRecursive(mux, pdMS_TO_TICKS(timeout)) // convenience macro that expands to xSemaphoreTakeRecursive - timeout is in milliseconds
775776
#define esp32SemGive(mux) xSemaphoreGiveRecursive(mux) // convenience macro that expands to xSemaphoreGiveRecursive
776777
#define WLED_create_spinlock(theSname) static portMUX_TYPE theSname = portMUX_INITIALIZER_UNLOCKED

0 commit comments

Comments
 (0)