From ffa3cbf3bace37c76f5c529c748d507ca2d9a784 Mon Sep 17 00:00:00 2001 From: Quique Tortosa Date: Tue, 1 Sep 2026 14:25:40 +0200 Subject: [PATCH 1/2] Ignore unknown keys in fromJson() instead of throwing find() throws std::runtime_error for keys that are not in the settings map, and fromJson() runs on the web server task with no try/catch around it, so an uncaught exception panics the device. A browser still holding a settings page from a different firmware version will post keys this build has never heard of - which is enough to reboot the display from the UI. Unknown keys are now logged and skipped. Also release the nvs handle on the validation-failure path, which returned without calling end(). Co-Authored-By: Claude Opus 5 --- src/JsonSettings.cpp | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/src/JsonSettings.cpp b/src/JsonSettings.cpp index fe41a7a1..4bc10c0e 100644 --- a/src/JsonSettings.cpp +++ b/src/JsonSettings.cpp @@ -103,11 +103,24 @@ bool JsonSettings::fromJson(JsonDocument settings) { for (JsonPair kv : settings.as()) { const char *key = kv.key().c_str(); + + // Skip keys we do not know about instead of letting find() throw. This + // body runs on the web server task, where an uncaught exception panics + // the whole device - and a browser still holding a page from a + // different firmware version will happily post keys we have never + // heard of. + if (this->map.find(key) == this->map.end()) { + Serial.print("Ignoring unknown setting: "); + Serial.println(key); + continue; + } + JsonSetting setting = this->find(key); if (! setting.validate(kv.value().as())) { lastValidationError = setting.getLastValidationError(); lastValidationKey = String(key); + preferences.end(); // do not leak the nvs handle on the error path return false; } From b75365f7b036758e399a9d963e9b1d354e7f012c Mon Sep 17 00:00:00 2001 From: Quique Tortosa Date: Tue, 1 Sep 2026 14:42:37 +0200 Subject: [PATCH 2/2] Serialize access to the settings store JsonSettings shares one Preferences instance between the Arduino loop task and the AsyncTCP task that serves the HTTP handlers, with no lock, and every accessor opens and closes NVS around a single get or put. getMode() runs on every pass through loop(), so the window is wide open: one task's end() closes the handle the other is part way through using. The read then falls back to the compiled default and the write is simply lost, while the request still answers "success" to the browser. Reproduced on an 8 module display: a POST /text returned success, mode read back as 0 (the compiled default for that key) and then reverted to its previous value, and the display never changed mode. A later POST /settings likewise reported success without storing anything; the same request succeeded on the second attempt. After this change, 12 of 12 consecutive write-then-read-back cycles matched. The mutex is recursive because reset() calls fromJson(toJson()) and all three take it. Co-Authored-By: Claude Opus 5 --- src/JsonSettings.cpp | 20 ++++++++++++++++++++ src/JsonSettings.h | 31 ++++++++++++++++++++++++++++++- 2 files changed, 50 insertions(+), 1 deletion(-) diff --git a/src/JsonSettings.cpp b/src/JsonSettings.cpp index 4bc10c0e..2156f170 100644 --- a/src/JsonSettings.cpp +++ b/src/JsonSettings.cpp @@ -4,6 +4,8 @@ #include String JsonSettings::getString(const char *key) { + Guard guard(mutex); + preferences.begin(name, true); String value = preferences.getString(key, this->find(key).strDefault); preferences.end(); @@ -11,6 +13,8 @@ String JsonSettings::getString(const char *key) { } int JsonSettings::getInt(const char *key) { + Guard guard(mutex); + preferences.begin(name, true); int value = preferences.getInt(key, this->find(key).intDefault); preferences.end(); @@ -18,6 +22,8 @@ int JsonSettings::getInt(const char *key) { } float JsonSettings::getFloat(const char *key) { + Guard guard(mutex); + preferences.begin(name, true); float value = preferences.getFloat(key, this->find(key).floatDefault); preferences.end(); @@ -25,6 +31,8 @@ float JsonSettings::getFloat(const char *key) { } std::vector JsonSettings::getIntVector(const char *key) { + Guard guard(mutex); + preferences.begin(name, true); String value = preferences.getString(key, this->find(key).strDefault); preferences.end(); @@ -45,18 +53,24 @@ std::vector JsonSettings::getIntVector(const char *key) { } void JsonSettings::putString(const char *key, String value) { + Guard guard(mutex); + preferences.begin(name, false); preferences.putString(key, value); preferences.end(); } void JsonSettings::putInt(const char *key, int value) { + Guard guard(mutex); + preferences.begin(name, false); preferences.putInt(key, value); preferences.end(); } void JsonSettings::putFloat(const char *key, float value) { + Guard guard(mutex); + preferences.begin(name, false); preferences.putFloat(key, value); preferences.end(); @@ -74,6 +88,8 @@ void JsonSettings::putIntVector(const char *key, std::vector value) { } JsonDocument JsonSettings::toJson() { + Guard guard(mutex); + JsonDocument settings; preferences.begin(name, true); @@ -99,6 +115,8 @@ JsonDocument JsonSettings::toJson() { } bool JsonSettings::fromJson(JsonDocument settings) { + Guard guard(mutex); + preferences.begin(name, false); for (JsonPair kv : settings.as()) { @@ -138,6 +156,8 @@ bool JsonSettings::fromJson(JsonDocument settings) { } bool JsonSettings::reset() { + Guard guard(mutex); + preferences.begin("config", false); preferences.clear(); preferences.end(); diff --git a/src/JsonSettings.h b/src/JsonSettings.h index 2de9f973..d33b7c1b 100644 --- a/src/JsonSettings.h +++ b/src/JsonSettings.h @@ -5,11 +5,14 @@ #include #include #include +#include +#include #include class JsonSettings { public: - JsonSettings(const char *name, std::map map) : name(name), map(map) {} + JsonSettings(const char *name, std::map map) + : name(name), map(map), mutex(xSemaphoreCreateRecursiveMutex()) {} String getString(const char *key); int getInt(const char *key); @@ -37,5 +40,31 @@ class JsonSettings { JsonSetting find(const char *key); + // Every accessor below opens and closes NVS on the single `preferences` + // instance. The Arduino loop task reads settings (getMode() runs on every + // pass through loop()) while the AsyncTCP task writes them from the HTTP + // handlers, so without this lock one task's end() closes the handle the + // other is mid-way through using: reads silently fall back to the compiled + // default and writes are lost while the request still answers "success". + // + // Recursive because reset() calls fromJson(toJson()), and both take it. + SemaphoreHandle_t mutex; + + class Guard { + public: + explicit Guard(SemaphoreHandle_t mutex) : mutex(mutex) { + xSemaphoreTakeRecursive(mutex, portMAX_DELAY); + } + ~Guard() { + xSemaphoreGiveRecursive(mutex); + } + + Guard(const Guard &) = delete; + Guard &operator=(const Guard &) = delete; + + private: + SemaphoreHandle_t mutex; + }; + Preferences preferences; };