diff --git a/modules/core_geocaching/tests/runtime_fakes/runtime_environment.h b/modules/core_geocaching/tests/runtime_fakes/runtime_environment.h index 4cfbadc4..842dfade 100644 --- a/modules/core_geocaching/tests/runtime_fakes/runtime_environment.h +++ b/modules/core_geocaching/tests/runtime_fakes/runtime_environment.h @@ -23,6 +23,8 @@ inline bool memory_available = true, card_ready = true, external_owner = false; inline size_t internal_free = SIZE_MAX; inline bool fail_read = false, in_ui = false; inline unsigned busy_reads = 0; +inline unsigned file_busy_cycles = 0; +inline std::array file_busy_remaining{}, file_busy_hits{}; inline std::string fail_read_path; inline size_t io_bytes = 0, ui_io = 0, blocked_io = 0; inline uint64_t io_operations = 0; diff --git a/modules/core_geocaching/tests/runtime_fakes/sd_runtime.cpp b/modules/core_geocaching/tests/runtime_fakes/sd_runtime.cpp index c7b73172..fb1c2bef 100644 --- a/modules/core_geocaching/tests/runtime_fakes/sd_runtime.cpp +++ b/modules/core_geocaching/tests/runtime_fakes/sd_runtime.cpp @@ -4,6 +4,18 @@ namespace platform::esp::arduino_common::storage { using namespace runtime_test; +bool fileBusy(unsigned operation) +{ + if (!file_busy_cycles) return false; + if (file_busy_remaining[operation] < file_busy_cycles) + { + ++file_busy_remaining[operation]; + ++file_busy_hits[operation]; + return true; + } + file_busy_remaining[operation] = 0; + return false; +} bool sd_card_ready() { return card_ready; } bool sd_external_block_owner_active() { return external_owner; } bool sd_is_directory(const char* path) { return access() && directories.count(path); } @@ -81,6 +93,8 @@ SdRuntimeFile::~SdRuntimeFile() bool SdRuntimeFile::open(const char* path, const char* mode) { close(); + read_busy_ = fileBusy(0); + if (read_busy_) return false; if (!access()) return false; impl_->path = path; impl_->offset = 0; @@ -100,15 +114,24 @@ void SdRuntimeFile::close() impl_->opened = false; } bool SdRuntimeFile::is_open() const { return impl_->opened; } -uint64_t SdRuntimeFile::size() const { return impl_->opened ? files.at(impl_->path).size() : 0; } +uint64_t SdRuntimeFile::size() const +{ + read_busy_ = fileBusy(1); + if (read_busy_) return 0; + return impl_->opened ? files.at(impl_->path).size() : 0; +} bool SdRuntimeFile::seek(uint64_t offset) { - if (!access() || !impl_->opened || offset > size()) return false; + read_busy_ = fileBusy(2); + if (read_busy_) return false; + if (!access() || !impl_->opened || offset > files.at(impl_->path).size()) return false; impl_->offset = static_cast(offset); return true; } int SdRuntimeFile::read(void* output, size_t capacity) { + read_busy_ = fileBusy(3); + if (read_busy_) return -1; if (!access() || !impl_->opened) return -1; if (fail_read || fail_read_path == impl_->path) { diff --git a/modules/core_geocaching/tests/test_index_scan_history.cpp b/modules/core_geocaching/tests/test_index_scan_history.cpp index eb63942c..010f1613 100644 --- a/modules/core_geocaching/tests/test_index_scan_history.cpp +++ b/modules/core_geocaching/tests/test_index_scan_history.cpp @@ -100,6 +100,13 @@ int main() const auto fallback = scan(144, false); const auto optimized = scan(frame.size(), false); require(optimized * 2 < fallback, "adjacent obsolete references still repeat full lookups"); + // Hold every open/size/seek/read busy across multiple worker turns. Neither + // scan nor lookup may advance its cursor before the operation succeeds. + test::file_busy_cycles = 3; + scan(frame.size(), false); + scan(144, false); + for (auto hits : test::file_busy_hits) require(hits > 0, "file operation contention was not exercised"); + test::file_busy_cycles = 0; char path[80]; require(sd::indexShardPathForBucket(root.slot, 11, bucket, path, sizeof(path)), "shard path"); test::files.at(path)[148] ^= 1; diff --git a/platform/esp/arduino_common/include/platform/esp/arduino_common/geocaching/sd_index_lookup.h b/platform/esp/arduino_common/include/platform/esp/arduino_common/geocaching/sd_index_lookup.h index 1951600c..a5028035 100644 --- a/platform/esp/arduino_common/include/platform/esp/arduino_common/geocaching/sd_index_lookup.h +++ b/platform/esp/arduino_common/include/platform/esp/arduino_common/geocaching/sd_index_lookup.h @@ -72,12 +72,16 @@ class SdIndexLookup } // Avoid the old one-byte whole-file probe: normal shards always // exceeded it, logged Invalid, then had to be opened a second time. - phase_ = file_.open(path, "r") ? Phase::CheckLength : Phase::Probe; + const bool opened = file_.open(path, "r"); + if (file_.read_busy()) return result_; + phase_ = opened ? Phase::CheckLength : Phase::Probe; return result_; } if (phase_ == Phase::CheckLength) { - if (file_.size() < length_) return fail(IndexLookupStep::Invalid); + const auto size = file_.size(); + if (file_.read_busy()) return result_; + if (size < length_) return fail(IndexLookupStep::Invalid); phase_ = Phase::Seek; return result_; } @@ -89,8 +93,8 @@ class SdIndexLookup phase_ = Phase::CheckSize; return result_; } + if (!file_.seek(position_ - kIndexEntrySize)) return file_.read_busy() ? result_ : fail(IndexLookupStep::IoError); position_ -= kIndexEntrySize; - if (!file_.seek(position_)) return fail(IndexLookupStep::IoError); read_ = 0; phase_ = Phase::Read; return result_; @@ -98,6 +102,7 @@ class SdIndexLookup if (phase_ == Phase::Read) { const int count = file_.read(bytes_.data() + read_, bytes_.size() - read_); + if (file_.read_busy()) return result_; if (count < 0) return fail(IndexLookupStep::IoError); if (!count || static_cast(count) > bytes_.size() - read_) return fail(IndexLookupStep::Invalid); read_ += static_cast(count); @@ -122,7 +127,9 @@ class SdIndexLookup } if (phase_ == Phase::CheckSize) { - if (file_.size() < length_) return fail(IndexLookupStep::Invalid); + const auto size = file_.size(); + if (file_.read_busy()) return result_; + if (size < length_) return fail(IndexLookupStep::Invalid); phase_ = Phase::Close; return result_; } diff --git a/platform/esp/arduino_common/include/platform/esp/arduino_common/geocaching/sd_index_scan.h b/platform/esp/arduino_common/include/platform/esp/arduino_common/geocaching/sd_index_scan.h index 71c124c4..371f9280 100644 --- a/platform/esp/arduino_common/include/platform/esp/arduino_common/geocaching/sd_index_scan.h +++ b/platform/esp/arduino_common/include/platform/esp/arduino_common/geocaching/sd_index_scan.h @@ -90,13 +90,16 @@ class SdIndexScan if (phase_ == Phase::Open) { char path[80]; - if (!indexShardPathForBucket(root_.slot, table_, static_cast(bucket_), path, sizeof(path)) || !file_.open(path, "r")) return fail(IndexScanStep::IoError); + if (!indexShardPathForBucket(root_.slot, table_, static_cast(bucket_), path, sizeof(path))) return fail(IndexScanStep::Invalid); + if (!file_.open(path, "r")) return file_.read_busy() ? result_ : fail(IndexScanStep::IoError); phase_ = Phase::Size; return result_; } if (phase_ == Phase::Size) { - if (file_.size() < head_.length) return fail(IndexScanStep::Invalid); + const auto size = file_.size(); + if (file_.read_busy()) return result_; + if (size < head_.length) return fail(IndexScanStep::Invalid); position_ = head_.length; newer_ = UINT64_MAX; phase_ = Phase::Seek; @@ -109,8 +112,8 @@ class SdIndexScan phase_ = Phase::EndShard; return result_; } + if (!file_.seek(position_ - kIndexEntrySize)) return file_.read_busy() ? result_ : fail(IndexScanStep::IoError); position_ -= kIndexEntrySize; - if (!file_.seek(position_)) return fail(IndexScanStep::IoError); read_ = 0; phase_ = Phase::Read; return result_; @@ -122,6 +125,7 @@ class SdIndexScan // the next one. Small callers retain the original lookup path. auto* incoming = capacity_ >= bytes_.size() ? frame_ : bytes_.data(); const int count = file_.read(incoming + read_, bytes_.size() - read_); + if (file_.read_busy()) return result_; if (count < 0) return fail(IndexScanStep::IoError); if (!count || static_cast(count) > bytes_.size() - read_) return fail(IndexScanStep::Invalid); read_ += static_cast(count); @@ -188,7 +192,9 @@ class SdIndexScan } if (phase_ == Phase::EndShard) { - if (file_.size() < head_.length) return fail(IndexScanStep::Invalid); + const auto size = file_.size(); + if (file_.read_busy()) return result_; + if (size < head_.length) return fail(IndexScanStep::Invalid); file_.close(); ++bucket_; phase_ = Phase::Bucket; diff --git a/platform/esp/arduino_common/include/platform/esp/arduino_common/geocaching/sd_indexed_value_reader.h b/platform/esp/arduino_common/include/platform/esp/arduino_common/geocaching/sd_indexed_value_reader.h index 59615788..a2c2e9ae 100644 --- a/platform/esp/arduino_common/include/platform/esp/arduino_common/geocaching/sd_indexed_value_reader.h +++ b/platform/esp/arduino_common/include/platform/esp/arduino_common/geocaching/sd_indexed_value_reader.h @@ -63,13 +63,14 @@ class SdIndexedValueReader static_cast(location_.segment_first_sequence)); else std::snprintf(path, sizeof(path), "/trailmate/geocaching/.state/checkpoint/%c.gcs", location_.source == ::geocaching::storage::IndexedValueSource::CheckpointA ? 'a' : 'b'); - if (!file_.open(path, "r")) return fail(IndexedReadStep::IoError); + if (!file_.open(path, "r")) return file_.read_busy() ? result_ : fail(IndexedReadStep::IoError); phase_ = Phase::Size; return result_; } if (phase_ == Phase::Size) { length_ = file_.size(); + if (file_.read_busy()) return result_; const uint64_t limit = location_.source == ::geocaching::storage::IndexedValueSource::Journal ? 1024U * 1024U : UINT32_MAX; if (!length_ || length_ > limit || location_.frame_offset >= length_) return fail(IndexedReadStep::Invalid); phase_ = Phase::Seek; @@ -77,7 +78,7 @@ class SdIndexedValueReader } if (phase_ == Phase::Seek) { - if (!file_.seek(location_.frame_offset)) return fail(IndexedReadStep::IoError); + if (!file_.seek(location_.frame_offset)) return file_.read_busy() ? result_ : fail(IndexedReadStep::IoError); offset_ = location_.frame_offset; phase_ = Phase::Read; return result_; @@ -127,7 +128,9 @@ class SdIndexedValueReader } if (phase_ == Phase::CheckSize) { - if (file_.size() != length_) return fail(IndexedReadStep::Invalid); + const auto size = file_.size(); + if (file_.read_busy()) return result_; + if (size != length_) return fail(IndexedReadStep::Invalid); phase_ = Phase::Close; return result_; } diff --git a/platform/esp/arduino_common/include/platform/esp/arduino_common/geocaching/sd_record_reader.h b/platform/esp/arduino_common/include/platform/esp/arduino_common/geocaching/sd_record_reader.h index a9fa2048..8644de01 100644 --- a/platform/esp/arduino_common/include/platform/esp/arduino_common/geocaching/sd_record_reader.h +++ b/platform/esp/arduino_common/include/platform/esp/arduino_common/geocaching/sd_record_reader.h @@ -47,6 +47,7 @@ inline SegmentReadResult readSdRecord(storage::SdRuntimeFile& file, uint64_t len const size_t needed = 24 - cursor.header_read; if (needed > length - offset) return SegmentReadResult::Truncated; const int n = file.read(buffer + cursor.header_read, needed); + if (file.read_busy()) return SegmentReadResult::InProgress; if (n == 0) return SegmentReadResult::Truncated; if (n < 0 || static_cast(n) > needed) return SegmentReadResult::IoError; cursor.header_read += static_cast(n); @@ -71,6 +72,7 @@ inline SegmentReadResult readSdRecord(storage::SdRuntimeFile& file, uint64_t len const size_t requested = std::min(budget, remaining); auto* destination = buffer + 24 + cursor.payload_read; const int n = file.read(destination, requested); + if (file.read_busy()) return SegmentReadResult::InProgress; if (n == 0) return SegmentReadResult::Truncated; if (n < 0 || static_cast(n) > requested) return SegmentReadResult::IoError; cursor.crc = ::sys::crc32(destination, static_cast(n), cursor.crc); diff --git a/platform/esp/arduino_common/include/platform/esp/arduino_common/storage/sd_card_runtime.h b/platform/esp/arduino_common/include/platform/esp/arduino_common/storage/sd_card_runtime.h index 210f5a5e..25b36aed 100644 --- a/platform/esp/arduino_common/include/platform/esp/arduino_common/storage/sd_card_runtime.h +++ b/platform/esp/arduino_common/include/platform/esp/arduino_common/storage/sd_card_runtime.h @@ -171,11 +171,15 @@ class SdRuntimeFile bool seek(uint64_t offset); uint64_t position() const; uint64_t size() const; + // Only describes the most recent open/read/seek/size call. Busy leaves + // the requested operation incomplete; callers may yield and try again. + bool read_busy() const { return read_busy_; } bool flush(); private: class Impl; Impl* impl_; + mutable bool read_busy_ = false; }; enum class SdDirReadStatus : uint8_t diff --git a/platform/esp/arduino_common/src/storage/sd_card_runtime.cpp b/platform/esp/arduino_common/src/storage/sd_card_runtime.cpp index d56408d3..76c9ae10 100644 --- a/platform/esp/arduino_common/src/storage/sd_card_runtime.cpp +++ b/platform/esp/arduino_common/src/storage/sd_card_runtime.cpp @@ -1406,6 +1406,7 @@ bool SdRuntimeFile::open(const char* path, const char* mode) bool SdRuntimeFile::open(const char* path, const char* mode, uint32_t expected_session) { close(); + read_busy_ = false; if (impl_ == nullptr || path_empty(path)) { return false; @@ -1428,6 +1429,7 @@ bool SdRuntimeFile::open(const char* path, const char* mode, uint32_t expected_s mutating ? kSdDurableLockWaitMs : kSdRuntimeLockWaitMs); if (!guard.locked()) { + read_busy_ = guard.status() == sys::runtime::BusAcquireStatus::Busy || guard.status() == sys::runtime::BusAcquireStatus::TimedOut; sd_io_end("file_open", impl_->path, start_ms, false, 0, -2); return false; } @@ -1441,6 +1443,7 @@ bool SdRuntimeFile::open(const char* path, const char* mode, uint32_t expected_s } impl_->session = expected_session; impl_->sdfat_file = s_sdfat.open(normalized, sdfat_open_flags(mode)); + read_busy_ = !impl_->sdfat_file && (guard.busStatus() == sys::runtime::BusAcquireStatus::Busy || guard.busStatus() == sys::runtime::BusAcquireStatus::TimedOut); impl_->backend = impl_->sdfat_file ? SdCardBackend::SdFat : SdCardBackend::None; sd_io_end("file_open", impl_->path, start_ms, impl_->backend == SdCardBackend::SdFat); return impl_->backend == SdCardBackend::SdFat; @@ -1510,6 +1513,7 @@ int SdRuntimeFile::available() const int SdRuntimeFile::read(void* buffer, std::size_t bytes_to_read) { + read_busy_ = false; if (!is_open() || buffer == nullptr || bytes_to_read == 0) { return 0; @@ -1520,6 +1524,7 @@ int SdRuntimeFile::read(void* buffer, std::size_t bytes_to_read) SdRuntimeOperationGuard guard("sd_file_read"); if (!guard.locked() || !is_open()) { + read_busy_ = !guard.locked() && (guard.status() == sys::runtime::BusAcquireStatus::Busy || guard.status() == sys::runtime::BusAcquireStatus::TimedOut); sd_io_end("file_read", impl_->path, start_ms, false, bytes_to_read, -2); return -1; } @@ -1532,6 +1537,7 @@ int SdRuntimeFile::read(void* buffer, std::size_t bytes_to_read) const int current = impl_->sdfat_file.read(out + total_read, slice); if (current <= 0) { + read_busy_ = total_read == 0 && (guard.busStatus() == sys::runtime::BusAcquireStatus::Busy || guard.busStatus() == sys::runtime::BusAcquireStatus::TimedOut); const int result = total_read > 0 ? static_cast(total_read) : current; sd_io_end("file_read", @@ -1771,6 +1777,7 @@ std::size_t SdRuntimeFile::printf(const char* format, ...) bool SdRuntimeFile::seek(uint64_t offset) { + read_busy_ = false; if (!is_open()) { return false; @@ -1780,9 +1787,12 @@ bool SdRuntimeFile::seek(uint64_t offset) SdRuntimeOperationGuard guard("sd_file_seek"); if (!guard.locked() || !is_open()) { + read_busy_ = !guard.locked() && (guard.status() == sys::runtime::BusAcquireStatus::Busy || guard.status() == sys::runtime::BusAcquireStatus::TimedOut); return false; } - return impl_->sdfat_file.seekSet(offset); + const bool result = impl_->sdfat_file.seekSet(offset); + read_busy_ = !result && (guard.busStatus() == sys::runtime::BusAcquireStatus::Busy || guard.busStatus() == sys::runtime::BusAcquireStatus::TimedOut); + return result; } return false; } @@ -1807,6 +1817,7 @@ uint64_t SdRuntimeFile::position() const uint64_t SdRuntimeFile::size() const { + read_busy_ = false; if (!is_open()) { return 0; @@ -1816,6 +1827,7 @@ uint64_t SdRuntimeFile::size() const SdRuntimeOperationGuard guard("sd_file_size"); if (!guard.locked() || !is_open()) { + read_busy_ = !guard.locked() && (guard.status() == sys::runtime::BusAcquireStatus::Busy || guard.status() == sys::runtime::BusAcquireStatus::TimedOut); return 0; } return impl_->sdfat_file.fileSize(); diff --git a/platform/esp/idf_common/src/sd_card_runtime_sdfat_adapter.cpp b/platform/esp/idf_common/src/sd_card_runtime_sdfat_adapter.cpp index d1d02516..ff435643 100644 --- a/platform/esp/idf_common/src/sd_card_runtime_sdfat_adapter.cpp +++ b/platform/esp/idf_common/src/sd_card_runtime_sdfat_adapter.cpp @@ -933,6 +933,7 @@ SdRuntimeFile::~SdRuntimeFile() bool SdRuntimeFile::open(const char* path, const char* mode) { close(); + read_busy_ = false; if (impl_ == nullptr || path_empty(path)) { return false; @@ -950,6 +951,7 @@ bool SdRuntimeFile::open(const char* path, const char* mode) SdRuntimeBusGuard guard("sd_file_open"); if (!guard.locked()) { + read_busy_ = s_storage_mutex != nullptr; sd_io_end("file_open", impl_->path, start_ms, false, 0, -2); return false; } @@ -1015,6 +1017,7 @@ int SdRuntimeFile::available() const int SdRuntimeFile::read(void* buffer, std::size_t bytes_to_read) { + read_busy_ = false; if (!is_open() || buffer == nullptr || bytes_to_read == 0) { return 0; @@ -1025,6 +1028,7 @@ int SdRuntimeFile::read(void* buffer, std::size_t bytes_to_read) SdRuntimeBusGuard guard("sd_file_read"); if (!guard.locked()) { + read_busy_ = s_storage_mutex != nullptr; sd_io_end("file_read", impl_->path, start_ms, false, bytes_to_read, -2); return -1; } @@ -1146,6 +1150,7 @@ std::size_t SdRuntimeFile::printf(const char* format, ...) bool SdRuntimeFile::seek(uint64_t offset) { + read_busy_ = false; if (!is_open()) { return false; @@ -1155,6 +1160,7 @@ bool SdRuntimeFile::seek(uint64_t offset) SdRuntimeBusGuard guard("sd_file_seek"); if (!guard.locked()) { + read_busy_ = s_storage_mutex != nullptr; return false; } return impl_->sdfat_file.seekSet(offset); @@ -1182,6 +1188,7 @@ uint64_t SdRuntimeFile::position() const uint64_t SdRuntimeFile::size() const { + read_busy_ = false; if (!is_open()) { return 0; @@ -1191,6 +1198,7 @@ uint64_t SdRuntimeFile::size() const SdRuntimeBusGuard guard("sd_file_size"); if (!guard.locked()) { + read_busy_ = s_storage_mutex != nullptr; return 0; } return impl_->sdfat_file.fileSize();