diff --git a/src/CrossPointSettings.cpp b/src/CrossPointSettings.cpp index 0b72d3a7..3b5b5dcb 100644 --- a/src/CrossPointSettings.cpp +++ b/src/CrossPointSettings.cpp @@ -6,6 +6,7 @@ #include #include +#include #include #include "I18nKeys.h" @@ -96,6 +97,7 @@ uint8_t CrossPointSettings::sleepTimeoutEnumToMinutes(const uint8_t legacyValue) } bool CrossPointSettings::saveToFile() const { + std::lock_guard lock(_mutex); Storage.mkdir("/.crosspoint"); return JsonSettingsIO::saveSettings(*this, SETTINGS_FILE_JSON); } @@ -106,7 +108,11 @@ bool CrossPointSettings::loadFromFile() { String json = Storage.readFile(SETTINGS_FILE_JSON); if (!json.isEmpty()) { bool resave = false; - bool result = JsonSettingsIO::loadSettings(*this, json.c_str(), &resave); + bool result; + { + std::lock_guard lock(_mutex); + result = JsonSettingsIO::loadSettings(*this, json.c_str(), &resave); + } if (result && resave) { if (saveToFile()) { LOG_DBG("CPS", "Resaved settings to update format"); @@ -166,6 +172,7 @@ bool CrossPointSettings::loadFromBinaryFile() { if (!Storage.openFileForRead("CPS", SETTINGS_FILE_BIN, inputFile)) { return false; } + std::lock_guard lock(_mutex); uint8_t version; serialization::readPod(inputFile, version); diff --git a/src/CrossPointSettings.h b/src/CrossPointSettings.h index 8a2cf217..717942cc 100644 --- a/src/CrossPointSettings.h +++ b/src/CrossPointSettings.h @@ -3,9 +3,12 @@ #include #include +#include class CrossPointSettings { private: + mutable std::mutex _mutex; + // Private constructor for singleton CrossPointSettings() = default; @@ -17,6 +20,10 @@ class CrossPointSettings { CrossPointSettings(const CrossPointSettings&) = delete; CrossPointSettings& operator=(const CrossPointSettings&) = delete; + // Access the settings mutex for protecting multi-field reads/writes from other cores. + // Callers must not re-enter SETTINGS methods that lock _mutex while holding it. + std::mutex& getMutex() const { return _mutex; } + enum SLEEP_SCREEN_MODE { DARK = 0, LIGHT = 1, diff --git a/src/CrossPointState.cpp b/src/CrossPointState.cpp index c90ce656..0a9d1a25 100644 --- a/src/CrossPointState.cpp +++ b/src/CrossPointState.cpp @@ -6,6 +6,7 @@ #include #include +#include namespace { constexpr uint8_t STATE_FILE_VERSION = 4; @@ -32,6 +33,7 @@ void CrossPointState::pushRecentSleep(uint16_t idx) { } bool CrossPointState::saveToFile() const { + std::lock_guard lock(_mutex); Storage.mkdir("/.crosspoint"); return JsonSettingsIO::saveState(*this, STATE_FILE_JSON); } @@ -41,6 +43,7 @@ bool CrossPointState::loadFromFile() { if (Storage.exists(STATE_FILE_JSON)) { String json = Storage.readFile(STATE_FILE_JSON); if (!json.isEmpty()) { + std::lock_guard lock(_mutex); return JsonSettingsIO::loadState(*this, json.c_str()); } } @@ -67,6 +70,7 @@ bool CrossPointState::loadFromBinaryFile() { if (!Storage.openFileForRead("CPS", STATE_FILE_BIN, inputFile)) { return false; } + std::lock_guard lock(_mutex); uint8_t version; serialization::readPod(inputFile, version); diff --git a/src/CrossPointState.h b/src/CrossPointState.h index 80d5d560..1a1a4cf7 100644 --- a/src/CrossPointState.h +++ b/src/CrossPointState.h @@ -1,12 +1,18 @@ #pragma once #include +#include #include class CrossPointState { + mutable std::mutex _mutex; + // Static instance static CrossPointState instance; public: + // Access the state mutex for protecting multi-field reads/writes from other cores. + std::mutex& getMutex() const { return _mutex; } + static constexpr uint8_t SLEEP_RECENT_COUNT = 16; std::string openEpubPath; diff --git a/src/activities/ActivityManager.cpp b/src/activities/ActivityManager.cpp index 96e85460..7757d050 100644 --- a/src/activities/ActivityManager.cpp +++ b/src/activities/ActivityManager.cpp @@ -19,12 +19,15 @@ #include "settings/SettingsActivity.h" #include "util/FullScreenMessageActivity.h" +static portMUX_TYPE activityManagerSpinlock = portMUX_INITIALIZER_UNLOCKED; + void ActivityManager::begin() { - xTaskCreate(&renderTaskTrampoline, "ActivityManagerRender", - 8192, // Stack size - this, // Parameters - 1, // Priority - &renderTaskHandle // Task handle + xTaskCreatePinnedToCore(&renderTaskTrampoline, "ActivityManagerRender", + 8192, // Stack size + this, // Parameters + 1, // Priority + &renderTaskHandle, // Task handle + 0 // Pin to core 0 (PRO_CPU) ); assert(renderTaskHandle != nullptr && "Failed to create render task"); } @@ -46,10 +49,10 @@ void ActivityManager::renderTaskLoop() { } // Notify any task blocked in requestUpdateAndWait() that the render is done. TaskHandle_t waiter = nullptr; - taskENTER_CRITICAL(nullptr); + taskENTER_CRITICAL(&activityManagerSpinlock); waiter = waitingTaskHandle; waitingTaskHandle = nullptr; - taskEXIT_CRITICAL(nullptr); + taskEXIT_CRITICAL(&activityManagerSpinlock); if (waiter) { xTaskNotify(waiter, 1, eIncrement); } @@ -137,8 +140,7 @@ void ActivityManager::loop() { } } - if (requestedUpdate) { - requestedUpdate = false; + if (requestedUpdate.exchange(false)) { // Using direct notification to signal the render task to update // Increment counter so multiple rapid calls won't be lost if (renderTaskHandle) { @@ -280,7 +282,7 @@ void ActivityManager::requestUpdateAndWait() { } // Atomic section to perform checks - taskENTER_CRITICAL(nullptr); + taskENTER_CRITICAL(&activityManagerSpinlock); auto currTaskHandler = xTaskGetCurrentTaskHandle(); auto mutexHolder = xSemaphoreGetMutexHolder(renderingMutex); bool isRenderTask = (currTaskHandler == renderTaskHandle); @@ -289,7 +291,7 @@ void ActivityManager::requestUpdateAndWait() { if (!alreadyWaiting && !isRenderTask && !holdingRenderLock) { waitingTaskHandle = currTaskHandler; } - taskEXIT_CRITICAL(nullptr); + taskEXIT_CRITICAL(&activityManagerSpinlock); // Render task cannot call requestUpdateAndWait() or it will cause a deadlock assert(!isRenderTask && "Render task cannot call requestUpdateAndWait()"); diff --git a/src/activities/ActivityManager.h b/src/activities/ActivityManager.h index 9066888d..5712ec71 100644 --- a/src/activities/ActivityManager.h +++ b/src/activities/ActivityManager.h @@ -4,6 +4,7 @@ #include #include +#include #include #include #include @@ -63,7 +64,7 @@ class ActivityManager { // Whether to trigger a render after the current loop() // This variable must only be set by the main loop, to avoid race conditions - bool requestedUpdate = false; + std::atomic requestedUpdate{false}; public: explicit ActivityManager(GfxRenderer& renderer, MappedInputManager& mappedInput)