From 3802679de43032ccefeec54d90d9f896558333f5 Mon Sep 17 00:00:00 2001 From: Ken Van Hoeylandt Date: Sun, 5 Oct 2025 16:16:55 +0200 Subject: [PATCH] File locking and DevelopmentService improvements (#358) - Moved `file::getlock(path)` from `Tactility` to `TactilityCore` - Changed all existing `file::*` functions to implement locking by default - Removed all manual locking where `file::*` functions were used - When `DevelopmentService` receives a file, it doesn't try to allocate it all in memory. This fixes going out-of-memory on devices without PSRAM. - Fix for TactilityC include --- Tactility/Include/Tactility/file/FileLock.h | 16 +---- .../Include/Tactility/network/HttpdReq.h | 2 + Tactility/Source/Tactility.cpp | 4 +- Tactility/Source/app/AppInstall.cpp | 46 +++++-------- Tactility/Source/app/files/State.cpp | 24 ++++--- Tactility/Source/app/files/View.cpp | 40 ++++++------ Tactility/Source/app/fileselection/State.cpp | 6 -- Tactility/Source/file/FileLock.cpp | 16 +---- Tactility/Source/file/PropertiesFile.cpp | 40 ++++++------ Tactility/Source/i18n/TextResources.cpp | 6 +- Tactility/Source/network/HttpdReq.cpp | 34 ++++++++++ .../development/DevelopmentService.cpp | 35 +++------- .../service/wifi/WifiBootSplashInit.cpp | 3 - Tactility/Source/settings/SystemSettings.cpp | 33 ++++------ TactilityC/Include/tt_app.h | 2 +- TactilityC/Source/tt_file.cpp | 2 +- TactilityCore/Include/Tactility/file/File.h | 30 +++++++++ TactilityCore/Source/file/File.cpp | 65 +++++++++++++++++-- 18 files changed, 224 insertions(+), 180 deletions(-) diff --git a/Tactility/Include/Tactility/file/FileLock.h b/Tactility/Include/Tactility/file/FileLock.h index c9687694..06fc62f2 100644 --- a/Tactility/Include/Tactility/file/FileLock.h +++ b/Tactility/Include/Tactility/file/FileLock.h @@ -13,20 +13,8 @@ namespace tt::file { /** * @param[in] path the path to find a lock for - * @return a non-null lock for the specified path. + * @return a lock instance when a lock was found, otherwise nullptr */ -std::shared_ptr getLock(const std::string& path); - -/** - * Acquires a lock, calls the function, then releases the lock. - * @param[in] path the path to find a lock for - * @param[in] fn the code to execute while the lock is acquired - */ -template -ReturnType withLock(const std::string& path, std::function fn) { - const auto lock = getLock(path)->asScopedLock(); - lock.lock(); - return fn(); -} +std::shared_ptr _Nullable findLock(const std::string& path); } diff --git a/Tactility/Include/Tactility/network/HttpdReq.h b/Tactility/Include/Tactility/network/HttpdReq.h index 0fbf9cac..9802f4f8 100644 --- a/Tactility/Include/Tactility/network/HttpdReq.h +++ b/Tactility/Include/Tactility/network/HttpdReq.h @@ -24,6 +24,8 @@ std::map parseContentDisposition(const std::vector #include +#include #ifdef ESP_PLATFORM #include @@ -188,11 +189,9 @@ static void registerInstalledApps(const std::string& path) { static void registerInstalledAppsFromSdCard(const std::shared_ptr& sdcard) { auto sdcard_root_path = sdcard->getMountPath(); auto app_path = std::format("{}/app", sdcard_root_path); - sdcard->getLock()->lock(); if (file::isDirectory(app_path)) { registerInstalledApps(app_path); } - sdcard->getLock()->unlock(); } static void registerInstalledAppsFromSdCards() { @@ -269,6 +268,7 @@ void run(const Configuration& config) { #ifdef ESP_PLATFORM initEsp(); #endif + file::setFindLockFunction(file::findLock); settings::initTimeZone(); hal::init(*config.hardware); network::ntp::init(); diff --git a/Tactility/Source/app/AppInstall.cpp b/Tactility/Source/app/AppInstall.cpp index 0f2a9034..31acbfeb 100644 --- a/Tactility/Source/app/AppInstall.cpp +++ b/Tactility/Source/app/AppInstall.cpp @@ -106,12 +106,9 @@ static bool untar(const std::string& tarPath, const std::string& destinationPath } void cleanupInstallDirectory(const std::string& path) { - const auto lock = file::getLock(path); - lock->lock(); if (!file::deleteRecursively(path)) { TT_LOG_W(TAG, "Failed to delete existing installation at %s", path.c_str()); } - lock->unlock(); } bool install(const std::string& path) { @@ -121,23 +118,18 @@ bool install(const std::string& path) { auto app_parent_path = getAppInstallPath(); TT_LOG_I(TAG, "Installing app %s to %s", path.c_str(), app_parent_path.c_str()); - auto target_path_lock = file::getLock(app_parent_path)->asScopedLock(); - - target_path_lock.lock(); auto filename = file::getLastPathSegment(path); const std::string app_target_path = std::format("{}/{}", app_parent_path, filename); if (file::isDirectory(app_target_path) && !file::deleteRecursively(app_target_path)) { TT_LOG_W(TAG, "Failed to delete %s", app_target_path.c_str()); } - target_path_lock.unlock(); - target_path_lock.lock(); if (!file::findOrCreateDirectory(app_target_path, 0777)) { TT_LOG_I(TAG, "Failed to create directory %s", app_target_path.c_str()); return false; } - target_path_lock.unlock(); + auto target_path_lock = file::getLock(app_parent_path)->asScopedLock(); auto source_path_lock = file::getLock(path)->asScopedLock(); target_path_lock.lock(); source_path_lock.lock(); @@ -149,23 +141,19 @@ bool install(const std::string& path) { source_path_lock.unlock(); target_path_lock.unlock(); - target_path_lock.lock(); auto manifest_path = app_target_path + "/manifest.properties"; if (!file::isFile(manifest_path)) { TT_LOG_E(TAG, "Manifest not found at %s", manifest_path.c_str()); cleanupInstallDirectory(app_target_path); return false; } - target_path_lock.unlock(); - target_path_lock.lock(); std::map properties; if (!file::loadPropertiesFile(manifest_path, properties)) { TT_LOG_E(TAG, "Failed to load manifest at %s", manifest_path.c_str()); cleanupInstallDirectory(app_target_path); return false; } - target_path_lock.unlock(); AppManifest manifest; if (!parseManifest(properties, manifest)) { @@ -179,7 +167,6 @@ bool install(const std::string& path) { stopAll(manifest.appId); } - target_path_lock.lock(); const std::string renamed_target_path = std::format("{}/{}", app_parent_path, manifest.appId); if (file::isDirectory(renamed_target_path)) { if (!file::deleteRecursively(renamed_target_path)) { @@ -188,15 +175,16 @@ bool install(const std::string& path) { return false; } } - target_path_lock.unlock(); target_path_lock.lock(); - if (rename(app_target_path.c_str(), renamed_target_path.c_str()) != 0) { + bool rename_success = rename(app_target_path.c_str(), renamed_target_path.c_str()) == 0; + target_path_lock.unlock(); + + if (!rename_success) { TT_LOG_E(TAG, "Failed to rename \"%s\" to \"%s\"", app_target_path.c_str(), manifest.appId.c_str()); cleanupInstallDirectory(app_target_path); return false; } - target_path_lock.unlock(); manifest.appLocation = Location::external(renamed_target_path); @@ -214,22 +202,20 @@ bool uninstall(const std::string& appId) { } auto app_path = getAppInstallPath(appId); - return file::withLock(app_path, [&app_path, &appId] { - if (!file::isDirectory(app_path)) { - TT_LOG_E(TAG, "App %s not found at ", app_path.c_str()); - return false; - } + if (!file::isDirectory(app_path)) { + TT_LOG_E(TAG, "App %s not found at ", app_path.c_str()); + return false; + } - if (!file::deleteRecursively(app_path)) { - return false; - } + if (!file::deleteRecursively(app_path)) { + return false; + } - if (!removeApp(appId)) { - TT_LOG_W(TAG, "Failed to remove app %d from registry", appId.c_str()); - } + if (!removeApp(appId)) { + TT_LOG_W(TAG, "Failed to remove app %d from registry", appId.c_str()); + } - return true; - }); + return true; } } // namespace \ No newline at end of file diff --git a/Tactility/Source/app/files/State.cpp b/Tactility/Source/app/files/State.cpp index 2933e21f..a4b7b6bc 100644 --- a/Tactility/Source/app/files/State.cpp +++ b/Tactility/Source/app/files/State.cpp @@ -57,19 +57,17 @@ bool State::setEntriesForPath(const std::string& path) { return true; } else { dir_entries.clear(); - return file::withLock(path, [this, &path] { - int count = file::scandir(path, dir_entries, &file::direntFilterDotEntries, file::direntSortAlphaAndType); - if (count >= 0) { - TT_LOG_I(TAG, "%s has %u entries", path.c_str(), count); - current_path = path; - selected_child_entry = ""; - action = ActionNone; - return true; - } else { - TT_LOG_E(TAG, "Failed to fetch entries for %s", path.c_str()); - return false; - } - }); + int count = file::scandir(path, dir_entries, &file::direntFilterDotEntries, file::direntSortAlphaAndType); + if (count >= 0) { + TT_LOG_I(TAG, "%s has %u entries", path.c_str(), count); + current_path = path; + selected_child_entry = ""; + action = ActionNone; + return true; + } else { + TT_LOG_E(TAG, "Failed to fetch entries for %s", path.c_str()); + return false; + } } } diff --git a/Tactility/Source/app/files/View.cpp b/Tactility/Source/app/files/View.cpp index dd29b450..31da848a 100644 --- a/Tactility/Source/app/files/View.cpp +++ b/Tactility/Source/app/files/View.cpp @@ -317,17 +317,18 @@ void View::onResult(LaunchId launchId, Result result, std::unique_ptr bu switch (state->getPendingAction()) { case State::ActionDelete: { if (alertdialog::getResultIndex(*bundle) == 0) { - file::withLock(filepath, [&filepath] { - if (file::isDirectory(filepath)) { - if (!file::deleteRecursively(filepath)) { - TT_LOG_W(TAG, "Failed to delete %s", filepath.c_str()); - } - } else if (file::isFile(filepath)) { - if (remove(filepath.c_str()) <= 0) { - TT_LOG_W(TAG, "Failed to delete %s", filepath.c_str()); - } - } - }); + if (file::isDirectory(filepath)) { + if (!file::deleteRecursively(filepath)) { + TT_LOG_W(TAG, "Failed to delete %s", filepath.c_str()); + } + } else if (file::isFile(filepath)) { + auto lock = file::getLock(filepath); + lock->lock(); + if (remove(filepath.c_str()) <= 0) { + TT_LOG_W(TAG, "Failed to delete %s", filepath.c_str()); + } + lock->unlock(); + } state->setEntriesForPath(state->getCurrentPath()); update(); @@ -337,14 +338,15 @@ 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()) { - file::withLock(filepath, [this, &filepath, &new_name] { - std::string rename_to = file::getChildPath(state->getCurrentPath(), new_name); - if (rename(filepath.c_str(), rename_to.c_str())) { - TT_LOG_I(TAG, "Renamed \"%s\" to \"%s\"", filepath.c_str(), rename_to.c_str()); - } else { - TT_LOG_E(TAG, "Failed to rename \"%s\" to \"%s\"", filepath.c_str(), rename_to.c_str()); - } - }); + auto lock = file::getLock(filepath); + lock->lock(); + std::string rename_to = file::getChildPath(state->getCurrentPath(), new_name); + if (rename(filepath.c_str(), rename_to.c_str())) { + TT_LOG_I(TAG, "Renamed \"%s\" to \"%s\"", filepath.c_str(), rename_to.c_str()); + } else { + TT_LOG_E(TAG, "Failed to rename \"%s\" to \"%s\"", filepath.c_str(), rename_to.c_str()); + } + lock->unlock(); state->setEntriesForPath(state->getCurrentPath()); update(); diff --git a/Tactility/Source/app/fileselection/State.cpp b/Tactility/Source/app/fileselection/State.cpp index b0e26fcc..52f40756 100644 --- a/Tactility/Source/app/fileselection/State.cpp +++ b/Tactility/Source/app/fileselection/State.cpp @@ -34,12 +34,6 @@ std::string State::getSelectedChildPath() const { } bool State::setEntriesForPath(const std::string& path) { - auto lock = mutex.asScopedLock(); - if (!lock.lock(100)) { - TT_LOG_E(TAG, LOG_MESSAGE_MUTEX_LOCK_FAILED_FMT, "setEntriesForPath"); - return false; - } - TT_LOG_I(TAG, "Changing path: %s -> %s", current_path.c_str(), path.c_str()); /** diff --git a/Tactility/Source/file/FileLock.cpp b/Tactility/Source/file/FileLock.cpp index 6a01bdf3..be8dfcb1 100644 --- a/Tactility/Source/file/FileLock.cpp +++ b/Tactility/Source/file/FileLock.cpp @@ -5,20 +5,8 @@ namespace tt::file { -class NoLock : public Lock { - bool lock(TickType_t timeout) const override { return true; } - bool unlock() const override { return true; } -}; - -static std::shared_ptr noLock = std::make_shared(); - -std::shared_ptr getLock(const std::string& path) { - auto sdcard_lock = hal::sdcard::findSdCardLock(path); - if (sdcard_lock != nullptr) { - return sdcard_lock; - } else { - return noLock; - } +std::shared_ptr _Nullable findLock(const std::string& path) { + return hal::sdcard::findSdCardLock(path); } } diff --git a/Tactility/Source/file/PropertiesFile.cpp b/Tactility/Source/file/PropertiesFile.cpp index 01cac08d..7b908358 100644 --- a/Tactility/Source/file/PropertiesFile.cpp +++ b/Tactility/Source/file/PropertiesFile.cpp @@ -19,28 +19,26 @@ bool getKeyValuePair(const std::string& input, std::string& key, std::string& va } bool loadPropertiesFile(const std::string& filePath, std::function callback) { - return file::withLock(filePath, [&filePath, &callback] { - TT_LOG_I(TAG, "Reading properties file %s", filePath.c_str()); - uint16_t line_count = 0; - std::string key_prefix = ""; - return readLines(filePath, true, [&key_prefix, &line_count, &filePath, &callback](const std::string& line) { - line_count++; - std::string key, value; - auto trimmed_line = string::trim(line, " \t"); - if (!trimmed_line.starts_with("#")) { - if (trimmed_line.starts_with("[")) { - key_prefix = trimmed_line; - } else { - if (getKeyValuePair(trimmed_line, key, value)) { - std::string trimmed_key = key_prefix + string::trim(key, " \t"); - std::string trimmed_value = string::trim(value, " \t"); - callback(trimmed_key, trimmed_value); - } else { - TT_LOG_E(TAG, "Failed to parse line %d of %s", line_count, filePath.c_str()); - } - } + TT_LOG_I(TAG, "Reading properties file %s", filePath.c_str()); + uint16_t line_count = 0; + std::string key_prefix = ""; + return readLines(filePath, true, [&key_prefix, &line_count, &filePath, &callback](const std::string& line) { + line_count++; + std::string key, value; + auto trimmed_line = string::trim(line, " \t"); + if (!trimmed_line.starts_with("#")) { + if (trimmed_line.starts_with("[")) { + key_prefix = trimmed_line; + } else { + if (getKeyValuePair(trimmed_line, key, value)) { + std::string trimmed_key = key_prefix + string::trim(key, " \t"); + std::string trimmed_value = string::trim(value, " \t"); + callback(trimmed_key, trimmed_value); + } else { + TT_LOG_E(TAG, "Failed to parse line %d of %s", line_count, filePath.c_str()); + } } - }); + } }); } diff --git a/Tactility/Source/i18n/TextResources.cpp b/Tactility/Source/i18n/TextResources.cpp index eb3307d8..11ee4b49 100644 --- a/Tactility/Source/i18n/TextResources.cpp +++ b/Tactility/Source/i18n/TextResources.cpp @@ -64,10 +64,8 @@ bool TextResources::load() { return false; } - file::withLock(file_path, [&file_path, &new_data] { - file::readLines(file_path, true, [&new_data](const char* line) { - new_data.push_back(line); - }); + file::readLines(file_path, true, [&new_data](const char* line) { + new_data.push_back(line); }); if (new_data.empty()) { diff --git a/Tactility/Source/network/HttpdReq.cpp b/Tactility/Source/network/HttpdReq.cpp index dcc5933e..3dccd58b 100644 --- a/Tactility/Source/network/HttpdReq.cpp +++ b/Tactility/Source/network/HttpdReq.cpp @@ -5,6 +5,7 @@ #include #include #include +#include #ifdef ESP_PLATFORM @@ -161,6 +162,39 @@ bool readAndDiscardOrSendError(httpd_req_t* request, const std::string& toRead) return true; } +size_t receiveFile(httpd_req_t* request, size_t length, const std::string& filePath) { + constexpr auto BUFFER_SIZE = 512; + char buffer[BUFFER_SIZE]; + size_t bytes_received = 0; + + auto lock = file::getLock(filePath)->asScopedLock(); + lock.lock(); + + auto* file = fopen(filePath.c_str(), "wb"); + if (file == nullptr) { + TT_LOG_E(TAG, "Failed to open file for writing: %s", filePath.c_str()); + return 0; + } + + while (bytes_received < length) { + auto expected_chunk_size = std::min(BUFFER_SIZE, length - bytes_received); + size_t receive_chunk_size = httpd_req_recv(request, buffer, expected_chunk_size); + if (receive_chunk_size <= 0) { + TT_LOG_E(TAG, "Receive failed"); + break; + } + if (fwrite(buffer, 1, receive_chunk_size, file) != receive_chunk_size) { + TT_LOG_E(TAG, "Failed to write all bytes"); + break; + } + bytes_received += receive_chunk_size; + } + + // Write file + fclose(file); + return bytes_received; +} + } #endif // ESP_PLATFORM \ No newline at end of file diff --git a/Tactility/Source/service/development/DevelopmentService.cpp b/Tactility/Source/service/development/DevelopmentService.cpp index b8af034b..8ab32afa 100644 --- a/Tactility/Source/service/development/DevelopmentService.cpp +++ b/Tactility/Source/service/development/DevelopmentService.cpp @@ -243,45 +243,30 @@ esp_err_t DevelopmentService::handleAppInstall(httpd_req_t* request) { return ESP_FAIL; } - // Receive file - size_t content_read; - auto part_after_file = std::format("\r\n--{}--\r\n", boundary); - auto file_size = content_left - part_after_file.length(); - auto buffer = network::receiveByteArray(request, file_size, content_read); - if (content_read != file_size) { - httpd_resp_send_err(request, HTTPD_400_BAD_REQUEST, "Multipart form error: file data not received"); - return ESP_FAIL; - } - content_left -= content_read; + // Receive boundary + auto boundary_and_newlines_after_file = std::format("\r\n--{}--\r\n", boundary); + auto file_size = content_left - boundary_and_newlines_after_file.length(); + // Create tmp directory const std::string tmp_path = getTempPath(); - auto lock = file::getLock(tmp_path)->asScopedLock(); - - lock.lock(); if (!file::findOrCreateDirectory(tmp_path, 0777)) { httpd_resp_send_err(request, HTTPD_500_INTERNAL_SERVER_ERROR, "Failed to save file"); return ESP_FAIL; } - lock.unlock(); - // Write file - lock.lock(); auto file_path = std::format("{}/{}", tmp_path, filename_entry->second); - auto* file = fopen(file_path.c_str(), "wb"); - auto file_bytes_written = fwrite(buffer.get(), 1, file_size, file); - fclose(file); - if (file_bytes_written != file_size) { + if (network::receiveFile(request, file_size, file_path) != file_size) { httpd_resp_send_err(request, HTTPD_500_INTERNAL_SERVER_ERROR, "Failed to save file"); return ESP_FAIL; } - lock.unlock(); + content_left -= file_size; // Read and verify part - if (!network::readAndDiscardOrSendError(request, part_after_file)) { + if (!network::readAndDiscardOrSendError(request, boundary_and_newlines_after_file)) { return ESP_FAIL; } - content_left -= part_after_file.length(); + content_left -= boundary_and_newlines_after_file.length(); if (content_left != 0) { TT_LOG_W(TAG, "We have more bytes at the end of the request parsing?!"); @@ -292,11 +277,9 @@ esp_err_t DevelopmentService::handleAppInstall(httpd_req_t* request) { return ESP_FAIL; } - lock.lock(); - if (remove(file_path.c_str()) != 0) { + if (!file::deleteFile(file_path.c_str())) { TT_LOG_W(TAG, "Failed to delete %s", file_path.c_str()); } - lock.unlock(); TT_LOG_I(TAG, "[200] /app/install -> %s", file_path.c_str()); diff --git a/Tactility/Source/service/wifi/WifiBootSplashInit.cpp b/Tactility/Source/service/wifi/WifiBootSplashInit.cpp index eaea7999..3ab3e44a 100644 --- a/Tactility/Source/service/wifi/WifiBootSplashInit.cpp +++ b/Tactility/Source/service/wifi/WifiBootSplashInit.cpp @@ -82,8 +82,6 @@ static void importWifiAp(const std::string& filePath) { static void importWifiApSettings(std::shared_ptr sdcard) { auto path = file::getChildPath(sdcard->getMountPath(), "settings"); - auto lock = sdcard->getLock()->asScopedLock(); - lock.lock(); std::vector dirent_list; if (file::scandir(path, dirent_list, [](const dirent* entry) { switch (entry->d_type) { @@ -104,7 +102,6 @@ static void importWifiApSettings(std::shared_ptr sdca }, nullptr) == 0) { return; } - lock.unlock(); if (dirent_list.empty()) { TT_LOG_W(TAG, "No AP files found at %s", sdcard->getMountPath().c_str()); diff --git a/Tactility/Source/settings/SystemSettings.cpp b/Tactility/Source/settings/SystemSettings.cpp index dd657586..2a06f17c 100644 --- a/Tactility/Source/settings/SystemSettings.cpp +++ b/Tactility/Source/settings/SystemSettings.cpp @@ -6,6 +6,7 @@ #include #include +#include namespace tt::settings { @@ -20,9 +21,7 @@ static bool loadSystemSettingsFromFile(SystemSettings& properties) { auto file_path = std::format(FILE_PATH_FORMAT, file::MOUNT_POINT_DATA); TT_LOG_I(TAG, "System settings loading from %s", file_path.c_str()); std::map map; - if (!file::withLock(file_path, [&map, &file_path] { - return file::loadPropertiesFile(file_path, map); - })) { + if (!file::loadPropertiesFile(file_path, map)) { TT_LOG_E(TAG, "Failed to load %s", file_path.c_str()); return false; } @@ -46,9 +45,6 @@ static bool loadSystemSettingsFromFile(SystemSettings& properties) { } bool loadSystemSettings(SystemSettings& properties) { - auto scoped_lock = mutex.asScopedLock(); - scoped_lock.lock(); - if (!cached) { if (!loadSystemSettingsFromFile(cachedSettings)) { return false; @@ -61,24 +57,19 @@ bool loadSystemSettings(SystemSettings& properties) { } bool saveSystemSettings(const SystemSettings& properties) { - auto scoped_lock = mutex.asScopedLock(); - scoped_lock.lock(); - auto file_path = std::format(FILE_PATH_FORMAT, file::MOUNT_POINT_DATA); - return file::withLock(file_path, [&properties, &file_path] { - std::map map; - map["language"] = toString(properties.language); - map["timeFormat24h"] = properties.timeFormat24h ? "true" : "false"; + std::map map; + map["language"] = toString(properties.language); + map["timeFormat24h"] = properties.timeFormat24h ? "true" : "false"; - if (!file::savePropertiesFile(file_path, map)) { - TT_LOG_E(TAG, "Failed to save %s", file_path.c_str()); - return false; - } + if (!file::savePropertiesFile(file_path, map)) { + TT_LOG_E(TAG, "Failed to save %s", file_path.c_str()); + return false; + } - cachedSettings = properties; - cached = true; - return true; - }); + cachedSettings = properties; + cached = true; + return true; } } \ No newline at end of file diff --git a/TactilityC/Include/tt_app.h b/TactilityC/Include/tt_app.h index e7b8ded4..97593a1d 100644 --- a/TactilityC/Include/tt_app.h +++ b/TactilityC/Include/tt_app.h @@ -1,6 +1,6 @@ #pragma once -#include +#include #include "tt_app_manifest.h" diff --git a/TactilityC/Source/tt_file.cpp b/TactilityC/Source/tt_file.cpp index 76db9e37..0d4d8ae8 100644 --- a/TactilityC/Source/tt_file.cpp +++ b/TactilityC/Source/tt_file.cpp @@ -1,6 +1,6 @@ #include #include -#include +#include extern "C" { diff --git a/TactilityCore/Include/Tactility/file/File.h b/TactilityCore/Include/Tactility/file/File.h index e3bb94a3..e4e26001 100644 --- a/TactilityCore/Include/Tactility/file/File.h +++ b/TactilityCore/Include/Tactility/file/File.h @@ -1,3 +1,7 @@ +/** + * 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! + */ #pragma once #include "Tactility/TactilityCore.h" @@ -40,6 +44,28 @@ struct FileCloser { } }; +typedef std::function(const std::string&)> FindLockFunction; + +/** + * @param[in] path the path to get a lock for + * @return a lock instance (never null) + */ +std::shared_ptr getLock(const std::string& path); + +void setFindLockFunction(const FindLockFunction& function); + +/** + * Acquires a lock, calls the function, then releases the lock. + * @param[in] path the path to find a lock for + * @param[in] fn the code to execute while the lock is acquired + */ +template +ReturnType withLock(const std::string& path, std::function fn) { + const auto lock = getLock(path)->asScopedLock(); + lock.lock(); + return fn(); +} + long getSize(FILE* file); /** Read a file and return its data. @@ -73,6 +99,10 @@ bool findOrCreateParentDirectory(const std::string& path, mode_t mode); bool deleteRecursively(const std::string& path); +bool deleteFile(const std::string& path); + +bool deleteDirectory(const std::string& path); + /** * Concatenate a child path with a parent path, ensuring proper slash inbetween * @param basePath an absolute path with or without trailing "/" diff --git a/TactilityCore/Source/file/File.cpp b/TactilityCore/Source/file/File.cpp index ce15e5bc..596b0906 100644 --- a/TactilityCore/Source/file/File.cpp +++ b/TactilityCore/Source/file/File.cpp @@ -11,7 +11,33 @@ class SdCardDevice; namespace tt::file { -#define TAG "file" +constexpr auto* TAG = "file"; + +class NoLock final : public Lock { + bool lock(TickType_t timeout) const override { return true; } + bool unlock() const override { return true; } +}; + +static std::shared_ptr noLock = std::make_shared(); +static std::function(const std::string&)> findLockFunction = nullptr; + +std::shared_ptr getLock(const std::string& path) { + if (findLockFunction == nullptr) { + TT_LOG_W(TAG, "File lock function not set!"); + return noLock; + } + + auto lock = findLockFunction(path); + if (lock == nullptr) { + return noLock; + } + + return lock; +} + +void setFindLockFunction(const FindLockFunction& function) { + findLockFunction = function; +} std::string getChildPath(const std::string& basePath, const std::string& childPath) { // Postfix with "/" when the current path isn't "/" @@ -40,6 +66,9 @@ bool listDirectory( const std::string& path, std::function onEntry ) { + auto lock = getLock(path)->asScopedLock(); + lock.lock(); + TT_LOG_I(TAG, "listDir start %s", path.c_str()); DIR* dir = opendir(path.c_str()); if (dir == nullptr) { @@ -64,6 +93,9 @@ int scandir( ScandirFilter _Nullable filterMethod, ScandirSort _Nullable sortMethod ) { + auto lock = getLock(path)->asScopedLock(); + lock.lock(); + TT_LOG_I(TAG, "scandir start"); DIR* dir = opendir(path.c_str()); if (dir == nullptr) { @@ -187,6 +219,9 @@ bool writeString(const std::string& filepath, const std::string& content) { } static bool findOrCreateDirectoryInternal(std::string path, mode_t mode) { + auto lock = getLock(path)->asScopedLock(); + lock.lock(); + struct stat dir_stat; if (mkdir(path.c_str(), mode) == 0) { return true; @@ -277,6 +312,7 @@ bool deleteRecursively(const std::string& path) { TT_LOG_E(TAG, "Failed to scan directory %s", path.c_str()); return false; } + for (const auto& entry : entries) { auto child_path = path + "/" + entry.d_name; if (!deleteRecursively(child_path)) { @@ -284,10 +320,10 @@ bool deleteRecursively(const std::string& path) { } } TT_LOG_I(TAG, "Deleting %s", path.c_str()); - return rmdir(path.c_str()) == 0; + return deleteDirectory(path); } else if (isFile(path)) { TT_LOG_I(TAG, "Deleting %s", path.c_str()); - return remove(path.c_str()) == 0; + return deleteFile(path); } else if (path == "/" || path == "." || path == "..") { // No-op return true; @@ -297,17 +333,36 @@ bool deleteRecursively(const std::string& path) { } } +bool deleteFile(const std::string& path) { + auto lock = getLock(path)->asScopedLock(); + lock.lock(); + return remove(path.c_str()) == 0; +} + +bool deleteDirectory(const std::string& path) { + auto lock = getLock(path)->asScopedLock(); + lock.lock(); + return rmdir(path.c_str()) == 0; +} + bool isFile(const std::string& path) { + auto lock = getLock(path)->asScopedLock(); + lock.lock(); return access(path.c_str(), F_OK) == 0; } bool isDirectory(const std::string& path) { + auto lock = getLock(path)->asScopedLock(); + lock.lock(); 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* file = fopen(filepath.c_str(), "r"); +bool readLines(const std::string& filePath, bool stripNewLine, std::function callback) { + auto lock = getLock(filePath)->asScopedLock(); + lock.lock(); + + auto* file = fopen(filePath.c_str(), "r"); if (file == nullptr) { return false; }