diff --git a/Tactility/Include/Tactility/file/File.h b/Tactility/Include/Tactility/file/File.h index 2148cd2ae..feacbd8a8 100644 --- a/Tactility/Include/Tactility/file/File.h +++ b/Tactility/Include/Tactility/file/File.h @@ -1,20 +1,24 @@ /** * All functions in this file can be safely called without manually applying file locks. - * For calls to C stdlib APIs such as fopen(), always call file::getLock(path) first! + * For calls to C stdlib APIs such as fopen(), always lock with file::FileMutexGuard(path) first! */ #pragma once #include -#include + +#include #include #include +#include +#include +#include #include #include /** * @warning SD card access requires a locking mechanism: - * @warning When using this in the Tactility main project, use `file::getLock()` or `file::withLock()` + * @warning When using this in the Tactility main project, use `file::FileMutexGuard` */ namespace tt::file { @@ -46,10 +50,25 @@ struct FileCloser { }; /** - * @param[in] path the path to get a lock for - * @return a lock instance (never null) + * RAII lock over TactilityKernel's file_mutex.h for the file system mount that owns `path`. + * Locks in the constructor, unlocks in the destructor - no heap allocation, no virtual dispatch. */ -std::shared_ptr getLock(const std::string& path) __attribute__((deprecated("Use file_mutex.h from TactilityKernel"))); +class FileMutexGuard final { + FileMutex mutex {}; + +public: + explicit FileMutexGuard(const std::string& path) { + file_mutex_get(&mutex, path.c_str()); + file_mutex_lock(&mutex); + } + + ~FileMutexGuard() { + file_mutex_unlock(&mutex); + } + + FileMutexGuard(const FileMutexGuard&) = delete; + FileMutexGuard& operator=(const FileMutexGuard&) = delete; +}; long getSize(FILE* file); diff --git a/Tactility/Include/Tactility/file/ObjectFile.h b/Tactility/Include/Tactility/file/ObjectFile.h index 08d06a73a..661e17d79 100644 --- a/Tactility/Include/Tactility/file/ObjectFile.h +++ b/Tactility/Include/Tactility/file/ObjectFile.h @@ -2,12 +2,12 @@ #include +#include + #include -#include "FileLock.h" - /** - * @warning The functionality below does NOT safely acquire file locks. Use file::getLock() or file::withLock() when using the functionality below. + * @warning The functionality below does NOT safely acquire file locks. Use file::FileMutexGuard when using the functionality below. */ namespace tt::file { @@ -45,7 +45,7 @@ class ObjectFileWriter { const uint32_t recordSize; const uint32_t recordVersion; const bool append; - const std::shared_ptr lock; + FileMutex mutex {}; std::unique_ptr file; uint32_t recordsWritten = 0; @@ -56,9 +56,10 @@ public: filePath(std::move(filePath)), recordSize(recordSize), recordVersion(recordVersion), - append(append), - lock(getLock(filePath)) - {} + append(append) + { + file_mutex_get(&mutex, this->filePath.c_str()); + } ~ObjectFileWriter() { diff --git a/Tactility/Private/Tactility/app/files/State.h b/Tactility/Private/Tactility/app/files/State.h index a4b20a3f4..b619cab3a 100644 --- a/Tactility/Private/Tactility/app/files/State.h +++ b/Tactility/Private/Tactility/app/files/State.h @@ -7,6 +7,7 @@ #include #include #include +#include namespace tt::app::files { @@ -31,6 +32,8 @@ private: std::string selected_child_entry; PendingAction action = ActionNone; std::string pending_paste_dst; + struct stat pending_paste_dst_stat {}; + bool pending_paste_dst_stat_valid = false; std::string clipboard_path; bool clipboard_is_cut = false; bool clipboard_active = false; @@ -81,6 +84,23 @@ public: std::string getPendingPasteDst() const { return pending_paste_dst; } void setPendingPasteDst(const std::string& dst) { pending_paste_dst = dst; } + /** Snapshot dst's stat at confirm-dialog time, so it can be revalidated right before the destructive delete. */ + void setPendingPasteDstStat(const struct stat& st) { + pending_paste_dst_stat = st; + pending_paste_dst_stat_valid = true; + } + + void clearPendingPasteDstStat() { pending_paste_dst_stat_valid = false; } + + /** True if dst had no prior snapshot (nothing to overwrite) or `st` still matches it. */ + bool pendingPasteDstMatches(const struct stat& st) const { + return !pending_paste_dst_stat_valid || + (pending_paste_dst_stat.st_dev == st.st_dev && + pending_paste_dst_stat.st_ino == st.st_ino && + pending_paste_dst_stat.st_size == st.st_size && + pending_paste_dst_stat.st_mtime == st.st_mtime); + } + void setClipboard(const std::string& path, bool is_cut) { mutex.withLock([&] { clipboard_path = path; diff --git a/Tactility/Source/Tactility.cpp b/Tactility/Source/Tactility.cpp index a6f4ca9b6..ae7321c98 100644 --- a/Tactility/Source/Tactility.cpp +++ b/Tactility/Source/Tactility.cpp @@ -40,6 +40,7 @@ #include #include #include +#include #include #include #include @@ -323,9 +324,9 @@ void createTempDirectory() { auto data_path = getUserDataPath(); auto temp_path = std::format("{}/tmp", data_path); if (!file::isDirectory(temp_path)) { - auto lockable = file::getLock(data_path); - auto lock = lockable->asScopedLock(); - if (lock.lock(1000 / portTICK_PERIOD_MS)) { + FileMutex mutex; + file_mutex_get(&mutex, data_path.c_str()); + if (file_mutex_try_lock(&mutex, 1000 / portTICK_PERIOD_MS)) { if (!file::findOrCreateParentDirectory(temp_path, 0777)) { LOG_E(TAG, "Failed to create %s", data_path.c_str()); } else if (mkdir(temp_path.c_str(), 0777) == 0) { @@ -333,6 +334,7 @@ void createTempDirectory() { } else { LOG_E(TAG, "Failed to create %s", temp_path.c_str()); } + file_mutex_unlock(&mutex); } else { LOG_E(TAG, LOG_MESSAGE_MUTEX_LOCK_FAILED_FMT, data_path.c_str()); } diff --git a/Tactility/Source/app/AppInstall.cpp b/Tactility/Source/app/AppInstall.cpp index 9e5670522..7b6e05561 100644 --- a/Tactility/Source/app/AppInstall.cpp +++ b/Tactility/Source/app/AppInstall.cpp @@ -3,7 +3,6 @@ #include #include #include -#include #include #include @@ -14,6 +13,7 @@ #include #include +#include #include namespace tt::app { @@ -117,19 +117,21 @@ bool install(const std::string& path) { return false; } - auto target_path_lockable = file::getLock(app_parent_path); - auto source_path_lockable = file::getLock(path); - auto target_path_lock = target_path_lockable->asScopedLock(); - auto source_path_lock = source_path_lockable->asScopedLock(); - target_path_lock.lock(); - source_path_lock.lock(); + FileMutex target_path_mutex; + file_mutex_get(&target_path_mutex, app_parent_path.c_str()); + FileMutex source_path_mutex; + file_mutex_get(&source_path_mutex, path.c_str()); + + file_mutex_lock(&target_path_mutex); + file_mutex_lock(&source_path_mutex); LOG_I(TAG, "Extracting app from %s to %s", path.c_str(), app_target_path.c_str()); - if (!untar(path, app_target_path)) { + bool untar_success = untar(path, app_target_path); + file_mutex_unlock(&source_path_mutex); + file_mutex_unlock(&target_path_mutex); + if (!untar_success) { LOG_E(TAG, "Failed to extract"); return false; } - source_path_lock.unlock(); - target_path_lock.unlock(); auto manifest_path = app_target_path + "/manifest.properties"; if (!file::isFile(manifest_path)) { @@ -159,9 +161,9 @@ bool install(const std::string& path) { } } - target_path_lock.lock(); + file_mutex_lock(&target_path_mutex); bool rename_success = rename(app_target_path.c_str(), renamed_target_path.c_str()) == 0; - target_path_lock.unlock(); + file_mutex_unlock(&target_path_mutex); if (!rename_success) { LOG_E(TAG, R"(Failed to rename "%s" to "%s")", app_target_path.c_str(), manifest.appId.c_str()); diff --git a/Tactility/Source/app/ElfApp.cpp b/Tactility/Source/app/ElfApp.cpp index 42d3f0ba3..8597fe77d 100644 --- a/Tactility/Source/app/ElfApp.cpp +++ b/Tactility/Source/app/ElfApp.cpp @@ -75,9 +75,10 @@ private: assert(elfFileData == nullptr); size_t size = 0; - file::getLock(elf_path)->withLock([this, &elf_path, &size]{ + { + file::FileMutexGuard guard(elf_path); elfFileData = file::readBinary(elf_path, size); - }); + } if (elfFileData == nullptr) { return false; diff --git a/Tactility/Source/app/apphub/AppHubEntry.cpp b/Tactility/Source/app/apphub/AppHubEntry.cpp index 023d3593a..a8dce05d4 100644 --- a/Tactility/Source/app/apphub/AppHubEntry.cpp +++ b/Tactility/Source/app/apphub/AppHubEntry.cpp @@ -21,9 +21,7 @@ static bool parseEntry(const cJSON* object, AppHubEntry& entry) { } bool parseJson(const std::string& filePath, std::vector& entries) { - auto lockable = file::getLock(filePath); - auto lock = lockable->asScopedLock(); - lock.lock(); + file::FileMutexGuard guard(filePath); auto data = file::readString(filePath); if (data == nullptr) { diff --git a/Tactility/Source/app/files/View.cpp b/Tactility/Source/app/files/View.cpp index eaf6e9ac7..15ae3376a 100644 --- a/Tactility/Source/app/files/View.cpp +++ b/Tactility/Source/app/files/View.cpp @@ -13,6 +13,7 @@ #include #include #include +#include #include #include @@ -101,17 +102,21 @@ static void onPastePressedCallback(lv_event_t* event) { // region File helpers static bool copyFileContents(const std::string& src, const std::string& dst) { - auto src_lock = file::getLock(src); - auto dst_lock = file::getLock(dst); - const bool same_lock = (src_lock.get() == dst_lock.get()); + FileMutex src_mutex; + file_mutex_get(&src_mutex, src.c_str()); + FileMutex dst_mutex; + file_mutex_get(&dst_mutex, dst.c_str()); + const bool same_lock = (src_mutex.lock == dst_mutex.lock && + src_mutex.try_lock == dst_mutex.try_lock && + src_mutex.unlock == dst_mutex.unlock); auto unlock_all = [&] { - if (!same_lock) dst_lock->unlock(); - src_lock->unlock(); + if (!same_lock) file_mutex_unlock(&dst_mutex); + file_mutex_unlock(&src_mutex); }; - src_lock->lock(); - if (!same_lock) dst_lock->lock(); + file_mutex_lock(&src_mutex); + if (!same_lock) file_mutex_lock(&dst_mutex); FILE* in = fopen(src.c_str(), "rb"); if (in == nullptr) { @@ -155,11 +160,12 @@ static bool copyRecursive(const std::string& src, const std::string& dst) { // Process one entry at a time: release the device lock between iterations // so other SPI bus users aren't starved, and stop immediately on failure. - auto lock = file::getLock(src); - lock->lock(); + FileMutex mutex; + file_mutex_get(&mutex, src.c_str()); + file_mutex_lock(&mutex); DIR* dir = opendir(src.c_str()); if (!dir) { - lock->unlock(); + file_mutex_unlock(&mutex); file::deleteRecursively(dst); return false; } @@ -171,14 +177,14 @@ static bool copyRecursive(const std::string& src, const std::string& dst) { if (strcmp(entry->d_name, ".") == 0 || strcmp(entry->d_name, "..") == 0) continue; std::string name = entry->d_name; // copy before releasing lock - lock->unlock(); + file_mutex_unlock(&mutex); success = copyRecursive(file::getChildPath(src, name), file::getChildPath(dst, name)); - lock->lock(); + file_mutex_lock(&mutex); } closedir(dir); - lock->unlock(); + file_mutex_unlock(&mutex); if (!success) { file::deleteRecursively(dst); @@ -593,12 +599,10 @@ void View::onResult(LaunchId launchId, Result result, std::unique_ptr bu LOG_W(TAG, "Failed to delete %s", filepath.c_str()); } } else if (file::isFile(filepath)) { - auto lock = file::getLock(filepath); - lock->lock(); + file::FileMutexGuard guard(filepath); if (remove(filepath.c_str()) != 0) { LOG_W(TAG, "Failed to delete %s", filepath.c_str()); } - lock->unlock(); } state->setEntriesForPath(state->getCurrentPath()); @@ -609,23 +613,22 @@ void View::onResult(LaunchId launchId, Result result, std::unique_ptr bu case State::ActionRename: { auto new_name = inputdialog::getResult(*bundle); if (!new_name.empty() && new_name != state->getSelectedChildEntry()) { - auto lock = file::getLock(filepath); - lock->lock(); std::string rename_to = file::getChildPath(state->getCurrentPath(), new_name); - struct stat st; - if (stat(rename_to.c_str(), &st) == 0) { - LOG_W(TAG, "Rename: destination already exists: \"%s\"", rename_to.c_str()); - lock->unlock(); - state->setPendingAction(State::ActionNone); - alertdialog::start("Rename failed", "\"" + new_name + "\" already exists."); - break; + { + file::FileMutexGuard guard(filepath); + struct stat st; + if (stat(rename_to.c_str(), &st) == 0) { + LOG_W(TAG, "Rename: destination already exists: \"%s\"", rename_to.c_str()); + state->setPendingAction(State::ActionNone); + alertdialog::start("Rename failed", "\"" + new_name + "\" already exists."); + break; + } + if (rename(filepath.c_str(), rename_to.c_str()) == 0) { + LOG_I(TAG, "Renamed \"%s\" to \"%s\"", filepath.c_str(), rename_to.c_str()); + } else { + LOG_E(TAG, "Failed to rename \"%s\" to \"%s\"", filepath.c_str(), rename_to.c_str()); + } } - if (rename(filepath.c_str(), rename_to.c_str()) == 0) { - LOG_I(TAG, "Renamed \"%s\" to \"%s\"", filepath.c_str(), rename_to.c_str()); - } else { - LOG_E(TAG, "Failed to rename \"%s\" to \"%s\"", filepath.c_str(), rename_to.c_str()); - } - lock->unlock(); state->setEntriesForPath(state->getCurrentPath()); update(); @@ -637,24 +640,23 @@ void View::onResult(LaunchId launchId, Result result, std::unique_ptr bu if (!filename.empty()) { std::string new_file_path = file::getChildPath(state->getCurrentPath(), filename); - auto lock = file::getLock(new_file_path); - lock->lock(); + { + file::FileMutexGuard guard(new_file_path); - struct stat st; - if (stat(new_file_path.c_str(), &st) == 0) { - LOG_W(TAG, "File already exists: \"%s\"", new_file_path.c_str()); - lock->unlock(); - break; - } + struct stat st; + if (stat(new_file_path.c_str(), &st) == 0) { + LOG_W(TAG, "File already exists: \"%s\"", new_file_path.c_str()); + break; + } - FILE* new_file = fopen(new_file_path.c_str(), "w"); - if (new_file) { - fclose(new_file); - LOG_I(TAG, "Created file \"%s\"", new_file_path.c_str()); - } else { - LOG_E(TAG, "Failed to create file \"%s\"", new_file_path.c_str()); + FILE* new_file = fopen(new_file_path.c_str(), "w"); + if (new_file) { + fclose(new_file); + LOG_I(TAG, "Created file \"%s\"", new_file_path.c_str()); + } else { + LOG_E(TAG, "Failed to create file \"%s\"", new_file_path.c_str()); + } } - lock->unlock(); state->setEntriesForPath(state->getCurrentPath()); update(); @@ -666,22 +668,21 @@ void View::onResult(LaunchId launchId, Result result, std::unique_ptr bu if (!foldername.empty()) { std::string new_folder_path = file::getChildPath(state->getCurrentPath(), foldername); - auto lock = file::getLock(new_folder_path); - lock->lock(); + { + file::FileMutexGuard guard(new_folder_path); - struct stat st; - if (stat(new_folder_path.c_str(), &st) == 0) { - LOG_W(TAG, "Folder already exists: \"%s\"", new_folder_path.c_str()); - lock->unlock(); - break; - } + struct stat st; + if (stat(new_folder_path.c_str(), &st) == 0) { + LOG_W(TAG, "Folder already exists: \"%s\"", new_folder_path.c_str()); + break; + } - if (mkdir(new_folder_path.c_str(), 0755) == 0) { - LOG_I(TAG, "Created folder \"%s\"", new_folder_path.c_str()); - } else { - LOG_E(TAG, "Failed to create folder \"%s\"", new_folder_path.c_str()); + if (mkdir(new_folder_path.c_str(), 0755) == 0) { + LOG_I(TAG, "Created folder \"%s\"", new_folder_path.c_str()); + } else { + LOG_E(TAG, "Failed to create folder \"%s\"", new_folder_path.c_str()); + } } - lock->unlock(); state->setEntriesForPath(state->getCurrentPath()); update(); @@ -693,6 +694,30 @@ void View::onResult(LaunchId launchId, Result result, std::unique_ptr bu auto clipboard = state->getClipboard(); if (clipboard.has_value()) { std::string dst = state->getPendingPasteDst(); + + // dst was last checked before the dialog was shown; a writer could + // have replaced it while the user was looking at the confirmation. + // Revalidate right before the destructive delete so we only ever + // remove the exact file the user agreed to overwrite. + bool dst_unchanged; + { + file::FileMutexGuard guard(dst); + struct stat current_stat {}; + dst_unchanged = (stat(dst.c_str(), ¤t_stat) == 0) && + state->pendingPasteDstMatches(current_stat); + } + state->clearPendingPasteDstStat(); + + if (!dst_unchanged) { + LOG_W(TAG, "Overwrite: destination \"%s\" changed since confirmation, aborting", dst.c_str()); + state->setPendingAction(State::ActionNone); + alertdialog::start( + "Overwrite aborted", + "\"" + file::getLastPathSegment(dst) + "\" changed while the dialog was open. Please try again." + ); + break; + } + // Trade-off: dst is removed before the copy attempt. If doPaste // subsequently fails (e.g. source read error, out of space), the // original dst data is unrecoverable. Acceptable for an embedded @@ -709,6 +734,8 @@ void View::onResult(LaunchId launchId, Result result, std::unique_ptr bu ); } } + } else { + state->clearPendingPasteDstStat(); } break; } @@ -742,23 +769,28 @@ void View::onPastePressed() { std::string entry_name = file::getLastPathSegment(src); std::string dst = file::getChildPath(state->getCurrentPath(), entry_name); - // Note: getLock(src) guards the source path; the existence check below is - // against dst, so there is a TOCTOU gap — another writer could create dst - // between this check and the write inside doPaste. Acceptable on a - // single-user embedded device; locking dst instead would be more correct. + // Note: FileMutexGuard(src) guards the source path; the existence check below is + // against dst, so there is a TOCTOU gap between this check and the write inside + // doPaste. When dst exists, the overwrite-confirm path below re-validates dst's + // stat immediately before the destructive delete (see ActionPaste in onResult), + // closing the window that matters (the dialog being open). When dst does not + // exist here, doPaste's write can still race a concurrent creator; acceptable on + // a single-user embedded device. if (src == dst) { LOG_I(TAG, "Paste: source and destination are the same path, skipping"); return; } - auto lock = file::getLock(src); - lock->lock(); - struct stat st; - bool dst_exists = (stat(dst.c_str(), &st) == 0); - lock->unlock(); + bool dst_exists; + struct stat dst_stat {}; + { + file::FileMutexGuard guard(src); + dst_exists = (stat(dst.c_str(), &dst_stat) == 0); + } if (dst_exists) { state->setPendingPasteDst(dst); + state->setPendingPasteDstStat(dst_stat); state->setPendingAction(State::ActionPaste); const std::vector choices = {"Overwrite", "Cancel"}; alertdialog::start("File exists", "Overwrite \"" + entry_name + "\"?", choices); @@ -772,10 +804,10 @@ void View::doPaste(const std::string& src, bool is_cut, const std::string& dst) bool success = false; bool src_delete_failed = false; if (is_cut) { - auto lock = file::getLock(src); - lock->lock(); - success = (rename(src.c_str(), dst.c_str()) == 0); - lock->unlock(); + { + file::FileMutexGuard guard(src); + success = (rename(src.c_str(), dst.c_str()) == 0); + } if (!success) { // Fallback for cross-filesystem moves: copy then delete. // Only mark success if both halves succeed — if the source removal diff --git a/Tactility/Source/app/notes/Notes.cpp b/Tactility/Source/app/notes/Notes.cpp index 5914b7e95..09fad15b9 100644 --- a/Tactility/Source/app/notes/Notes.cpp +++ b/Tactility/Source/app/notes/Notes.cpp @@ -2,7 +2,6 @@ #include #include -#include #include #include @@ -83,29 +82,29 @@ class NotesApp final : public App { void openFile(const std::string& path) { // We might be reading from the SD card, which could share a SPI bus with other devices (display) - file::getLock(path)->withLock([this, path] { - auto data = file::readString(path); - if (data != nullptr) { - lvgl_lock(); - lv_textarea_set_text(uiNoteText, reinterpret_cast(data.get())); - lv_label_set_text(uiCurrentFileName, path.c_str()); - lvgl_unlock(); - filePath = path; - LOG_I(TAG, "Loaded from %s", path.c_str()); - } - }); + file::FileMutexGuard guard(path); + auto data = file::readString(path); + if (data != nullptr) { + lvgl_lock(); + lv_textarea_set_text(uiNoteText, reinterpret_cast(data.get())); + lv_label_set_text(uiCurrentFileName, path.c_str()); + lvgl_unlock(); + filePath = path; + LOG_I(TAG, "Loaded from %s", path.c_str()); + } } bool saveFile(const std::string& path) { // We might be writing to SD card, which could share a SPI bus with other devices (display) bool result = false; - file::getLock(path)->withLock([&result, this, path] { - if (file::writeString(path, saveBuffer.c_str())) { - LOG_I(TAG, "Saved to %s", path.c_str()); - filePath = path; - result = true; - } - }); + { + file::FileMutexGuard guard(path); + if (file::writeString(path, saveBuffer.c_str())) { + LOG_I(TAG, "Saved to %s", path.c_str()); + filePath = path; + result = true; + } + } return result; } diff --git a/Tactility/Source/file/File.cpp b/Tactility/Source/file/File.cpp index 1ad42792e..b800e725b 100644 --- a/Tactility/Source/file/File.cpp +++ b/Tactility/Source/file/File.cpp @@ -4,9 +4,7 @@ #include #include -#include #include -#include #include namespace tt::hal::sdcard { @@ -17,27 +15,6 @@ namespace tt::file { constexpr auto* TAG = "file"; -class FileMutexLock final : public Lock { - FileMutex mutex; - -public: - explicit FileMutexLock(const std::string& path) { - file_mutex_get(&mutex, path.c_str()); - } - - bool lock(TickType_t timeout) const override { - return file_mutex_try_lock(&mutex, timeout); - } - - void unlock() const override { - file_mutex_unlock(&mutex); - } -}; - -std::shared_ptr getLock(const std::string& path) { - return std::make_shared(path); -} - std::string getChildPath(const std::string& basePath, const std::string& childPath) { // Postfix with "/" when the current path isn't "/" if (basePath.length() != 1) { @@ -65,9 +42,7 @@ bool listDirectory( const std::string& path, std::function onEntry ) { - auto lockable = getLock(path); - auto lock = lockable->asScopedLock(); - lock.lock(); + FileMutexGuard guard(path); LOG_I(TAG, "listDir start %s", path.c_str()); DIR* dir = opendir(path.c_str()); @@ -93,9 +68,7 @@ int scandir( ScandirFilter filterMethod, ScandirSort sortMethod ) { - auto lockable = getLock(path); - auto lock = lockable->asScopedLock(); - lock.lock(); + FileMutexGuard guard(path); LOG_I(TAG, "scandir start"); DIR* dir = opendir(path.c_str()); @@ -220,9 +193,7 @@ bool writeString(const std::string& filepath, const std::string& content) { } static bool findOrCreateDirectoryInternal(std::string path, mode_t mode) { - auto lockable = getLock(path); - auto lock = lockable->asScopedLock(); - lock.lock(); + FileMutexGuard guard(path); struct stat dir_stat; if (mkdir(path.c_str(), mode) == 0) { @@ -336,38 +307,28 @@ bool deleteRecursively(const std::string& path) { } bool deleteFile(const std::string& path) { - auto lockable = getLock(path); - auto lock = lockable->asScopedLock(); - lock.lock(); + FileMutexGuard guard(path); return remove(path.c_str()) == 0; } bool deleteDirectory(const std::string& path) { - auto lockable = getLock(path); - auto lock = lockable->asScopedLock(); - lock.lock(); + FileMutexGuard guard(path); return rmdir(path.c_str()) == 0; } bool isFile(const std::string& path) { - auto lockable = getLock(path); - auto lock = lockable->asScopedLock(); - lock.lock(); + FileMutexGuard guard(path); return access(path.c_str(), F_OK) == 0; } bool isDirectory(const std::string& path) { - auto lockable = getLock(path); - auto lock = lockable->asScopedLock(); - lock.lock(); + FileMutexGuard guard(path); struct stat stat_result; return stat(path.c_str(), &stat_result) == 0 && S_ISDIR(stat_result.st_mode); } bool readLines(const std::string& filePath, bool stripNewLine, std::function callback) { - auto lockable = getLock(filePath); - auto lock = lockable->asScopedLock(); - lock.lock(); + FileMutexGuard guard(filePath); auto* file = fopen(filePath.c_str(), "r"); if (file == nullptr) { diff --git a/Tactility/Source/file/PropertiesFile.cpp b/Tactility/Source/file/PropertiesFile.cpp index 003b4ba72..3bd1d2974 100644 --- a/Tactility/Source/file/PropertiesFile.cpp +++ b/Tactility/Source/file/PropertiesFile.cpp @@ -2,7 +2,6 @@ #include #include -#include #include namespace tt::file { @@ -55,22 +54,20 @@ bool loadPropertiesFile(const std::string& filePath, std::map& properties) { - bool result = false; - getLock(filePath)->withLock([&result, filePath, &properties] { - LOG_I(TAG, "Saving properties file %s", filePath.c_str()); + FileMutexGuard guard(filePath); - FILE* file = fopen(filePath.c_str(), "w"); - if (file == nullptr) { - LOG_E(TAG, "Failed to open %s", filePath.c_str()); - return; - } + LOG_I(TAG, "Saving properties file %s", filePath.c_str()); - for (const auto& [key, value]: properties) { fprintf(file, "%s=%s\n", key.c_str(), value.c_str()); } + FILE* file = fopen(filePath.c_str(), "w"); + if (file == nullptr) { + LOG_E(TAG, "Failed to open %s", filePath.c_str()); + return false; + } - fclose(file); - result = true; - }); - return result; + for (const auto& [key, value]: properties) { fprintf(file, "%s=%s\n", key.c_str(), value.c_str()); } + + fclose(file); + return true; } } diff --git a/Tactility/Source/lvgl/LabelUtils.cpp b/Tactility/Source/lvgl/LabelUtils.cpp index 23493e6c7..ccd8a8628 100644 --- a/Tactility/Source/lvgl/LabelUtils.cpp +++ b/Tactility/Source/lvgl/LabelUtils.cpp @@ -1,14 +1,14 @@ #include #include -#include namespace tt::lvgl { bool label_set_text_file(lv_obj_t* label, const char* filepath) { std::unique_ptr text; - file::getLock(filepath)->withLock([&text, filepath] { + { + file::FileMutexGuard guard(filepath); text = file::readString(filepath); - }); + } if (text != nullptr) { lv_label_set_text(label, reinterpret_cast(text.get())); diff --git a/Tactility/Source/network/Http.cpp b/Tactility/Source/network/Http.cpp index 46748d482..6832d2639 100644 --- a/Tactility/Source/network/Http.cpp +++ b/Tactility/Source/network/Http.cpp @@ -70,9 +70,7 @@ void download( auto bytes_left = client->getContentLength(); - auto lockable = file::getLock(downloadFilePath); - auto lock = lockable->asScopedLock(); - lock.lock(); + file::FileMutexGuard guard(downloadFilePath); LOG_I(TAG, "opening %s", downloadFilePath.c_str()); auto* file = fopen(downloadFilePath.c_str(), "wb"); if (file == nullptr) { diff --git a/Tactility/Source/network/HttpdReq.cpp b/Tactility/Source/network/HttpdReq.cpp index 9d9c86324..d575f4ae0 100644 --- a/Tactility/Source/network/HttpdReq.cpp +++ b/Tactility/Source/network/HttpdReq.cpp @@ -186,9 +186,7 @@ size_t receiveFile(httpd_req_t* request, size_t length, const std::string& fileP char buffer[BUFFER_SIZE]; size_t bytes_received = 0; - auto lockable = file::getLock(filePath); - auto lock = lockable->asScopedLock(); - lock.lock(); + file::FileMutexGuard guard(filePath); auto* file = fopen(filePath.c_str(), "wb"); if (file == nullptr) { diff --git a/Tactility/Source/service/webserver/AssetVersion.cpp b/Tactility/Source/service/webserver/AssetVersion.cpp index db6555002..fc94677c4 100644 --- a/Tactility/Source/service/webserver/AssetVersion.cpp +++ b/Tactility/Source/service/webserver/AssetVersion.cpp @@ -30,13 +30,11 @@ static bool loadVersionFromFile(const char* path, AssetVersion& version) { // Read file content std::string content; { - auto lock = file::getLock(path); - lock->lock(portMAX_DELAY); + file::FileMutexGuard guard(path); FILE* fp = fopen(path, "r"); if (!fp) { LOG_E(TAG, "Failed to open version file: %s", path); - lock->unlock(); return false; } @@ -44,7 +42,6 @@ static bool loadVersionFromFile(const char* path, AssetVersion& version) { size_t bytesRead = fread(buffer, 1, sizeof(buffer) - 1, fp); bool readError = ferror(fp) != 0; fclose(fp); - lock->unlock(); if (readError) { LOG_E(TAG, "Error reading version file: %s", path); @@ -117,9 +114,8 @@ static bool saveVersionToFile(const char* path, const AssetVersion& version) { // Write to file bool success = false; { - auto lock = file::getLock(path); - lock->lock(portMAX_DELAY); - + file::FileMutexGuard guard(path); + FILE* fp = fopen(path, "w"); if (fp) { size_t len = strlen(jsonString); @@ -139,7 +135,6 @@ static bool saveVersionToFile(const char* path, const AssetVersion& version) { } fclose(fp); } - lock->unlock(); } cJSON_free(jsonString); diff --git a/Tactility/Source/service/webserver/WebServerService.cpp b/Tactility/Source/service/webserver/WebServerService.cpp index 6c6a62c11..93132bf07 100644 --- a/Tactility/Source/service/webserver/WebServerService.cpp +++ b/Tactility/Source/service/webserver/WebServerService.cpp @@ -1703,8 +1703,7 @@ esp_err_t WebServerService::handleAssets(httpd_req_t* request) { httpd_resp_set_type(request, "image/png"); httpd_resp_set_hdr(request, "Cache-Control", "public, max-age=86400"); - auto lock = file::getLock(faviconPath); - lock->lock(portMAX_DELAY); + file::FileMutexGuard guard(faviconPath); FILE* fp = fopen(faviconPath, "rb"); if (fp) { @@ -1713,17 +1712,14 @@ esp_err_t WebServerService::handleAssets(httpd_req_t* request) { while ((bytesRead = fread(buffer, 1, sizeof(buffer), fp)) > 0) { if (httpd_resp_send_chunk(request, buffer, bytesRead) != ESP_OK) { fclose(fp); - lock->unlock(); return ESP_FAIL; } } fclose(fp); - lock->unlock(); httpd_resp_send_chunk(request, nullptr, 0); LOG_I(TAG, "[200] %s (favicon)", uri); return ESP_OK; } - lock->unlock(); } // If favicon not found, return 404 silently (browsers handle this gracefully) httpd_resp_send_err(request, HTTPD_404_NOT_FOUND, "Not found"); @@ -1752,9 +1748,8 @@ esp_err_t WebServerService::handleAssets(httpd_req_t* request) { httpd_resp_set_type(request, getContentType(dataPath)); // Read and send file using standard C FILE* operations - auto lock = file::getLock(dataPath); - lock->lock(portMAX_DELAY); - + file::FileMutexGuard guard(dataPath); + FILE* fp = fopen(dataPath.c_str(), "rb"); if (fp) { char buffer[512]; @@ -1762,18 +1757,15 @@ esp_err_t WebServerService::handleAssets(httpd_req_t* request) { while ((bytesRead = fread(buffer, 1, sizeof(buffer), fp)) > 0) { if (httpd_resp_send_chunk(request, buffer, bytesRead) != ESP_OK) { fclose(fp); - lock->unlock(); return ESP_FAIL; } } fclose(fp); - lock->unlock(); httpd_resp_send_chunk(request, nullptr, 0); // End of chunks LOG_I(TAG, "[200] %s (from Data)", uri); return ESP_OK; } - lock->unlock(); } // Fallback to SD card @@ -1781,9 +1773,8 @@ esp_err_t WebServerService::handleAssets(httpd_req_t* request) { if (file::isFile(sdPath.c_str())) { httpd_resp_set_type(request, getContentType(sdPath)); - auto lock = file::getLock(sdPath); - lock->lock(portMAX_DELAY); - + file::FileMutexGuard guard(sdPath); + FILE* fp = fopen(sdPath.c_str(), "rb"); if (fp) { char buffer[512]; @@ -1791,18 +1782,15 @@ esp_err_t WebServerService::handleAssets(httpd_req_t* request) { while ((bytesRead = fread(buffer, 1, sizeof(buffer), fp)) > 0) { if (httpd_resp_send_chunk(request, buffer, bytesRead) != ESP_OK) { fclose(fp); - lock->unlock(); return ESP_FAIL; } } fclose(fp); - lock->unlock(); - + httpd_resp_send_chunk(request, nullptr, 0); // End of chunks LOG_I(TAG, "[200] %s (from SD)", uri); return ESP_OK; } - lock->unlock(); } // File not found