From feb7f0574bf9b284afb9df21af71bed97051a889 Mon Sep 17 00:00:00 2001 From: Torlando <281092095+torlando-agent[bot]@users.noreply.github.com> Date: Mon, 7 Sep 2026 18:52:03 +0000 Subject: [PATCH 1/3] fix(nomadnet): bound cache transient-stall so a failing SD seam cannot freeze the UI The SD page cache (16af3b5) made the SD card a soft dependency of NomadNet page loads, but its step machine retries a transient storage result (BUSY = SPI-mutex timeout, UNAVAILABLE = card not mounted) forever with no budget. On a persistently unhealthy seam the boot-time recovery pins operation_ != NONE, the flow stays LOOKUP, and the UI freezes at 'Checking SD page cache...' (the CACHE state has no deadline, unlike every other NomadNet state). NomadNetCache::service() now compares each call's entry state to the previous call's. Any advance (op, offset, scan/cleanup index, scan count, open-flags) resets a stall counter, so slow-but-progressing steps (chunked 64 KB transfers, up to 96-record directory scans) never false-trip; a no-progress tick is a transient stall. Past 500 consecutive no-progress ticks (far beyond any real SPI contention or SD mount window) the cache bails: mark the namespace non-authoritative for the session (lookups/commits bypass) and clear the op, so the flow falls through to a live fetch -- the pre-cache page-load behavior. Regression tests: cache-level (persistent UNAVAILABLE recovery bails; lookup bypasses after bail) and flow-level (a permanently BUSY beginList no longer parks the flow in LOOKUP; it reaches NEED_LIVE in bounded ticks). Both fail on the pre-fix code and pass with it. Verified: tdeck firmware build SUCCESS; tests/native 121 passed (3 pre-existing env failures, fail identically on origin/main baseline); tests/build_scripts 184 passed. --- lib/tdeck_ui/UI/LXMF/NomadNetCache.cpp | 56 +++++++++++++++++++++++ lib/tdeck_ui/UI/LXMF/NomadNetCache.h | 24 ++++++++++ tests/native/test_nomadnet_cache.cpp | 30 +++++++++++- tests/native/test_nomadnet_cache_flow.cpp | 16 ++++++- 4 files changed, 124 insertions(+), 2 deletions(-) diff --git a/lib/tdeck_ui/UI/LXMF/NomadNetCache.cpp b/lib/tdeck_ui/UI/LXMF/NomadNetCache.cpp index 76de69c5..3826c8d3 100644 --- a/lib/tdeck_ui/UI/LXMF/NomadNetCache.cpp +++ b/lib/tdeck_ui/UI/LXMF/NomadNetCache.cpp @@ -929,6 +929,38 @@ CacheResult NomadNetCache::invalidate(const CacheKey& key) { } void NomadNetCache::service() { + // Transient-stall guard (cross-call): compare this call's entry state to + // the previous call's. A transient (BUSY/UNAVAILABLE) retry changes none + // of the tracked bits, so an unchanged entry across consecutive service() + // calls is a stall; any advance resets the counter. Past the bounded + // budget, bail so a persistently unhealthy SD seam can never pin the cache + // (and the NomadNet UI at "Checking SD page cache..."). The baseline is + // refreshed before the switch so every path — including early returns — + // leaves a correct entry for the next call. + if (operation_ != Operation::NONE && + operation_ == transient_prev_op_ && + offset_ == transient_prev_offset_ && + scan_index_ == transient_prev_scan_index_ && + cleanup_index_ == transient_prev_cleanup_index_ && + scan_seen_ == transient_prev_scan_seen_ && + read_open_ == transient_prev_read_open_ && + write_open_ == transient_prev_write_open_) { + if (transient_stall_count_ < MAX_TRANSIENT_STALL_TICKS) { + ++transient_stall_count_; + } else { + transient_stall_count_ = 0; + transient_bail(); + } + } else if (operation_ != Operation::NONE) { + transient_stall_count_ = 0; + } + transient_prev_op_ = operation_; + transient_prev_offset_ = offset_; + transient_prev_scan_index_ = scan_index_; + transient_prev_cleanup_index_ = cleanup_index_; + transient_prev_scan_seen_ = scan_seen_; + transient_prev_read_open_ = read_open_; + transient_prev_write_open_ = write_open_; switch (operation_) { case Operation::NONE: return; @@ -1428,6 +1460,30 @@ void NomadNetCache::service() { } } +void NomadNetCache::transient_bail() { + // The storage seam has been stalled for far longer than any real SPI + // contention or SD mount window. Stop retrying: drop all namespace + // authority (lookups and commits now bypass), and clear the in-flight op + // so the caller's flow falls through to a live fetch, restoring the + // pre-cache page-load behavior instead of a frozen UI. + transient_stall_count_ = 0; + namespace_authoritative_ = false; + read_open_ = false; + write_open_ = false; + commit_job_ = false; + cleanup_stages_after_failure_ = false; + eviction_pending_ = false; + eviction_generation_ = -1; + quota_recovery_ = false; + recovery_complete_ = false; + offset_ = 0; + io_.clear(); + metadata_bytes_.clear(); + ExternalVector().swap(body_); + operation_ = Operation::NONE; + result_ = CacheResult::BYPASS; +} + void NomadNetCache::cancel() { if (!busy() || eviction_pending_ || quota_recovery_) { return; diff --git a/lib/tdeck_ui/UI/LXMF/NomadNetCache.h b/lib/tdeck_ui/UI/LXMF/NomadNetCache.h index 771267d6..c3d7f46c 100644 --- a/lib/tdeck_ui/UI/LXMF/NomadNetCache.h +++ b/lib/tdeck_ui/UI/LXMF/NomadNetCache.h @@ -226,6 +226,30 @@ private: CacheResult quota_result_ = CacheResult::STORED; bool quota_recovery_ = false; + // Transient-stall guard. A storage op that returns BUSY/UNAVAILABLE (SPI + // mutex starved, card not mounted) is retried forever by the step machine; + // on a persistently unhealthy seam that pins operation_ != NONE and the + // NomadNet UI freezes at "Checking SD page cache...". service() compares + // this call's entry state to the previous call's: any advance (op change, + // offset, scan/cleanup index, scan count, open-flag) resets the stall + // counter, so slow-but-progressing steps (chunked reads/writes, directory + // scans) never false-trip; a tick with no advance is a transient stall. + // Past the bounded budget the cache bails: mark the namespace + // non-authoritative for the session (lookups/commits then bypass) and + // clear the op, so the flow falls through to a live fetch (the pre-cache + // behavior). 500 no-progress ticks far exceeds any real SPI contention or + // SD mount window (each op already waits only 100 ms on the bus mutex). + static constexpr std::uint32_t MAX_TRANSIENT_STALL_TICKS = 500; + Operation transient_prev_op_ = Operation::NONE; + std::size_t transient_prev_offset_ = 0; + std::size_t transient_prev_scan_index_ = 0; + std::size_t transient_prev_cleanup_index_ = 0; + std::size_t transient_prev_scan_seen_ = 0; + bool transient_prev_read_open_ = false; + bool transient_prev_write_open_ = false; + std::uint32_t transient_stall_count_ = 0; + void transient_bail(); + static constexpr std::size_t VERIFY_SCRATCH_BYTES = 1024; std::array verify_scratch_{}; std::uint64_t verify_hash_ = 1469598103934665603ULL; diff --git a/tests/native/test_nomadnet_cache.cpp b/tests/native/test_nomadnet_cache.cpp index a1b353bd..24e58729 100644 --- a/tests/native/test_nomadnet_cache.cpp +++ b/tests/native/test_nomadnet_cache.cpp @@ -26,6 +26,25 @@ struct MemoryStorage final:NomadNetStorage{ StorageResult endList()override{++operations;return StorageResult::OK;}std::vectorlist;size_t li=0; }; static CacheKey key(const char*path="/page/index.mu"){return CacheKey{"0123456789abcdef0123456789abcdef",path,RequestDataClass::NIL};} +// A seam whose directory enumeration is permanently BUSY (SPI mutex starved +// for longer than any real contention window) — the exact condition that +// pinned the boot-time recovery and froze the UI at "Checking SD page cache...". +struct HangListStorage final:NomadNetStorage{ + StorageResult hang = StorageResult::OK; + bool isAvailable()const override{return true;} + StorageResult beginRead(const char*,uint32_t&s)override{s=0;return StorageResult::MISS;} + StorageResult readChunk(uint8_t*,size_t,size_t&n)override{n=0;return StorageResult::OK;} + StorageResult endRead()override{return StorageResult::OK;} + StorageResult beginWrite(const char*)override{return StorageResult::OK;} + StorageResult writeChunk(const uint8_t*,size_t z,size_t&n)override{n=z;return StorageResult::OK;} + StorageResult commitWrite()override{return StorageResult::OK;} + StorageResult abortWrite()override{return StorageResult::OK;} + StorageResult remove(const char*)override{return StorageResult::MISS;} + StorageResult rename(const char*,const char*)override{return StorageResult::MISS;} + StorageResult stat(const char*,uint32_t&)override{return StorageResult::MISS;} + StorageResult beginList(const char*)override{return hang;} + StorageResult nextList(char*,size_t,bool&d)override{d=true;return StorageResult::OK;} + StorageResult endList()override{return StorageResult::OK;}}; static void drain(NomadNetCache&c,MemoryStorage*s=nullptr){for(int i=0;i<1000&&c.busy();++i){const auto before=s?s->operations:0;c.service();if(s&&s->operations-before>1)throw std::runtime_error("more than one storage operation per service tick");}} int main(){int f=0;auto ck=[&](bool x,const char*n){if(!x){++f;std::cerr<<"FAIL "< body={'h','e','l','l','o'}; ck(canonical_cache_key(key())=="0123456789abcdef0123456789abcdef\n/page/index.mu\nnil","canonical key"); @@ -45,7 +64,16 @@ int main(){int f=0;auto ck=[&](bool x,const char*n){if(!x){++f;std::cerr<<"FAIL // Interrupted promotion keeps a valid prior generation. MemoryStorage s2;NomadNetCache c2(s2,cfg);drain(c2,&s2);ck(c2.beginCommit(key(),body,300,60)==CacheResult::PENDING,"seed");drain(c2);s2.fail_rename_at=2;ck(c2.beginCommit(key(),std::vector{'x'},301,60)==CacheResult::PENDING,"crash commit");drain(c2);ck(c2.lastResult()==CacheResult::STORAGE_ERROR,"rename crash reported");s2.fail_rename_at=0;ck(c2.beginLookup(key(),302)==CacheResult::PENDING,"fallback after crash");drain(c2);ck(c2.takeBody(got)&&std::equal(got.begin(),got.end(),body.begin(),body.end()),"prior generation survives"); // SD faults are cache misses to caller, never page failures. - s2.available=false;ck(c2.beginLookup(key(),302)==CacheResult::PENDING,"unavailable begins");drain(c2);ck(c2.lastResult()==CacheResult::BYPASS,"unavailable bypass");s2.available=true;s2.busy=true;ck(c2.beginLookup(key(),302)==CacheResult::PENDING,"busy begins");drain(c2);ck(c2.lastResult()==CacheResult::BYPASS,"busy bypass"); + s2.available=false;ck(c2.beginLookup(key(),302)==CacheResult::PENDING,"unavailable begins");drain(c2);ck(c2.lastResult()==CacheResult::BYPASS,"unavailable bypass");s2.available=true;s2.busy=true;ck(c2.beginLookup(key(),302)==CacheResult::PENDING,"busy begins");drain(c2);ck(c2.lastResult()==CacheResult::BYPASS,"busy bypass");s2.busy=false; + // A persistently transient seam during boot-time recovery must not pin the + // cache (and the UI). Before this fix the recovery retried beginList forever; + // now the stall budget expires, the namespace is marked non-authoritative, + // and the op clears so the flow can fall through to a live fetch. + MemoryStorage busy_boot;busy_boot.available=false;NomadNetCache bc(busy_boot,cfg); + for(int i=0;i<2000&&bc.busy();++i)bc.service(); + ck(!bc.busy(),"persistent transient recovery bails instead of pinning"); + ck(!bc.recoveryComplete(),"bailed recovery is not authoritative"); + ck(bc.beginLookup(key(),100)==CacheResult::BYPASS,"post-bail lookup bypasses to live"); // Deterministic expired-first then oldest quota eviction. MemoryStorage s3;NomadNetCache c3(s3,cfg);drain(c3,&s3);for(int i=0;i<3;i++){auto k=key((std::string("/page/")+char('a'+i)).c_str());ck(c3.beginCommit(k,body,400+i,i==0?1:100)==CacheResult::PENDING,"quota commit");drain(c3);}ck(c3.entryCount()<=2&&c3.totalBytes()<=cfg.max_bytes,"quotas bounded");ck(c3.beginLookup(key("/page/a"),500)==CacheResult::PENDING,"evicted lookup");drain(c3);ck(c3.lastResult()!=CacheResult::HIT,"expired evicted first"); diff --git a/tests/native/test_nomadnet_cache_flow.cpp b/tests/native/test_nomadnet_cache_flow.cpp index c802ad0d..27d4d92f 100644 --- a/tests/native/test_nomadnet_cache_flow.cpp +++ b/tests/native/test_nomadnet_cache_flow.cpp @@ -2,9 +2,10 @@ #include #include #include +#include "NomadNetCache.h" #include "NomadNetCacheFlow.h" using namespace UI::LXMF::NomadNet; -struct Mem:NomadNetStorage{std::map>f;std::string a;size_t p=0;bool w=false;bool isAvailable()const override{return true;}StorageResult beginRead(const char*n,uint32_t&s)override{auto i=f.find(n);if(i==f.end())return StorageResult::MISS;a=n;p=0;s=i->second.size();return StorageResult::OK;}StorageResult readChunk(uint8_t*o,size_t c,size_t&n)override{auto&v=f[a];n=std::min(c,v.size()-p);memcpy(o,v.data()+p,n);p+=n;return StorageResult::OK;}StorageResult endRead()override{return StorageResult::OK;}StorageResult beginWrite(const char*n)override{a=n;f[a].clear();w=true;return StorageResult::OK;}StorageResult writeChunk(const uint8_t*d,size_t z,size_t&n)override{n=z;f[a].insert(f[a].end(),d,d+z);return StorageResult::OK;}StorageResult commitWrite()override{w=false;return StorageResult::OK;}StorageResult abortWrite()override{w=false;return StorageResult::OK;}StorageResult remove(const char*n)override{return f.erase(n)?StorageResult::OK:StorageResult::MISS;}StorageResult rename(const char*x,const char*y)override{auto i=f.find(x);if(i==f.end())return StorageResult::MISS;f[y]=i->second;f.erase(i);return StorageResult::OK;}StorageResult stat(const char*,uint32_t&)override{return StorageResult::MISS;}StorageResult beginList(const char*)override{return StorageResult::OK;}StorageResult nextList(char*,size_t,bool&d)override{d=true;return StorageResult::OK;}StorageResult endList()override{return StorageResult::OK;}}; +struct Mem:NomadNetStorage{std::map>f;std::string a;size_t p=0;bool w=false;bool list_busy=false;bool isAvailable()const override{return true;}StorageResult beginRead(const char*n,uint32_t&s)override{auto i=f.find(n);if(i==f.end())return StorageResult::MISS;a=n;p=0;s=i->second.size();return StorageResult::OK;}StorageResult readChunk(uint8_t*o,size_t c,size_t&n)override{auto&v=f[a];n=std::min(c,v.size()-p);memcpy(o,v.data()+p,n);p+=n;return StorageResult::OK;}StorageResult endRead()override{return StorageResult::OK;}StorageResult beginWrite(const char*n)override{a=n;f[a].clear();w=true;return StorageResult::OK;}StorageResult writeChunk(const uint8_t*d,size_t z,size_t&n)override{n=z;f[a].insert(f[a].end(),d,d+z);return StorageResult::OK;}StorageResult commitWrite()override{w=false;return StorageResult::OK;}StorageResult abortWrite()override{w=false;return StorageResult::OK;}StorageResult remove(const char*n)override{return f.erase(n)?StorageResult::OK:StorageResult::MISS;}StorageResult rename(const char*x,const char*y)override{auto i=f.find(x);if(i==f.end())return StorageResult::MISS;f[y]=i->second;f.erase(i);return StorageResult::OK;}StorageResult stat(const char*,uint32_t&)override{return StorageResult::MISS;}StorageResult beginList(const char*)override{return list_busy?StorageResult::BUSY:StorageResult::OK;}StorageResult nextList(char*,size_t,bool&d)override{d=true;return StorageResult::OK;}StorageResult endList()override{return StorageResult::OK;}}; int main(){int f=0;auto ck=[&](bool x,const char*n){if(!x){f++;std::cerr<<"FAIL "<b={'o','k'};CacheEligibility e{true,true,false,false,false,false,RequestDataClass::NIL};ck(flow.acceptLive(b,e,100),"valid live accepted");ck(flow.pageReady()&&flow.status()=="Page loaded (live)","render ready before commit");for(int i=0;i<20;i++)flow.service(); NomadNetCacheFlow hit(c);ck(hit.begin(k,101,false)==CacheFlowState::LOOKUP,"second lookup");for(int i=0;i<10&&hit.state()==CacheFlowState::LOOKUP;i++)hit.service();ExternalVectorout;ck(hit.state()==CacheFlowState::READY&&hit.takePage(out)&&std::equal(out.begin(),out.end(),b.begin(),b.end())&&hit.status()=="Cached page; current reachability not checked","hit without peer and without internal-vector copy"); @@ -27,4 +28,17 @@ int main(){int f=0;auto ck=[&](bool x,const char*n){if(!x){f++;std::cerr<<"FAIL ck(recovering_reload.state()==CacheFlowState::NEED_LIVE,"reload starts exactly one live fetch only after terminal invalidation"); ck(recovering.beginLookup(k,102)==CacheResult::PENDING,"post reload invalidation lookup");for(int i=0;i<50&&recovering.busy();++i)recovering.service(); ck(recovering.lastResult()!=CacheResult::HIT,"reload during recovery removed stale generation"); + // The device symptom: boot-time recovery pinned by a permanently BUSY SD + // seam (SPI mutex starved). Before the fix the flow sat in LOOKUP forever + // and the UI froze at "Checking SD page cache..."; now the stall budget + // expires and the flow falls through to a live fetch. + { + Mem hang;hang.list_busy=true;NomadNetCache hc(hang);NomadNetCacheFlow hf(hc); + CacheKey hk{"fedcba9876543210fedcba9876543210","/page/hang.mu",RequestDataClass::NIL}; + ck(hf.begin(hk,1000,false)==CacheFlowState::LOOKUP,"hang lookup admitted"); + int serviced=0; + while(hf.state()==CacheFlowState::LOOKUP&&serviced<10000){hf.service();++serviced;} + ck(hf.state()==CacheFlowState::NEED_LIVE,"pinned recovery no longer freezes the lookup"); + ck(serviced<10000,"lookup reached live in bounded service ticks"); + } std::cout<<(f?"failed":"passed")<<"\n";return f?1:0;} From 32a09938d898746b75c51e1d38cc5b4c6ae0ed3d Mon Sep 17 00:00:00 2001 From: Torlando <281092095+torlando-agent[bot]@users.noreply.github.com> Date: Tue, 15 Sep 2026 06:27:14 +0000 Subject: [PATCH 2/3] fix(nomadnet): address Greploop round 1 on the cache transient-stall guard Three findings on the transient-stall bail, all valid: - Reload invalidation: a bail recorded BYPASS, but NomadNetCacheFlow::service() accepts only MISS as a successful invalidation, so a bail during an admitted reload reported 'Page cache invalidation failed' instead of falling through to a live fetch. The bail now records MISS. - Open-resource leak: the bail cleared read_open_/write_open_ (and abandoned an open directory enumeration) without calling endRead()/abortWrite()/endList(), leaking SD handles. The bail now releases each in-flight resource via the seam's own teardown (bounded best-effort; a still-transient close is accepted rather than re-pinning the op). - Tick-vs-time: the not-ready UNAVAILABLE path returns immediately (no bus wait), so a pure 500-tick budget could expire during a legitimate SD mount window and disable caching for the whole session. The bail is now gated on BOTH the tick floor AND a 10s wall-time window (service() takes a monotonic ms clock; production passes millis(), 0 is a safe default for tests). Regression tests: reload-invalidation bail -> NEED_LIVE (flow), flat-clock does not bail, healable transient keeps authority, list-open (RECOVERY_END) stall bails and releases the handle (cache). --- lib/tdeck_ui/UI/LXMF/NomadNetCache.cpp | 69 +++++++++++++++++----- lib/tdeck_ui/UI/LXMF/NomadNetCache.h | 26 ++++++-- lib/tdeck_ui/UI/LXMF/NomadNetCacheFlow.cpp | 4 +- lib/tdeck_ui/UI/LXMF/NomadNetCacheFlow.h | 2 +- lib/tdeck_ui/UI/LXMF/UIManager.cpp | 2 +- tests/native/test_nomadnet_cache.cpp | 37 +++++++++++- tests/native/test_nomadnet_cache_flow.cpp | 27 ++++++++- 7 files changed, 137 insertions(+), 30 deletions(-) diff --git a/lib/tdeck_ui/UI/LXMF/NomadNetCache.cpp b/lib/tdeck_ui/UI/LXMF/NomadNetCache.cpp index 3826c8d3..6b9ea245 100644 --- a/lib/tdeck_ui/UI/LXMF/NomadNetCache.cpp +++ b/lib/tdeck_ui/UI/LXMF/NomadNetCache.cpp @@ -928,15 +928,19 @@ CacheResult NomadNetCache::invalidate(const CacheKey& key) { return result_; } -void NomadNetCache::service() { +void NomadNetCache::service(std::uint64_t now_ms) { // Transient-stall guard (cross-call): compare this call's entry state to // the previous call's. A transient (BUSY/UNAVAILABLE) retry changes none // of the tracked bits, so an unchanged entry across consecutive service() - // calls is a stall; any advance resets the counter. Past the bounded - // budget, bail so a persistently unhealthy SD seam can never pin the cache - // (and the NomadNet UI at "Checking SD page cache..."). The baseline is - // refreshed before the switch so every path — including early returns — - // leaves a correct entry for the next call. + // calls is a stall; any advance resets the counter. The bail is gated on + // BOTH a tick floor and an elapsed-time window: a not-ready SD seam + // returns UNAVAILABLE immediately (no bus wait), so a pure tick budget + // could expire during a legitimate mount window and disable caching for + // the whole session. Past both bounds, bail so a persistently unhealthy + // SD seam can never pin the cache (and the NomadNet UI at "Checking SD + // page cache..."). The baseline is refreshed before the switch so every + // path — including early returns — leaves a correct entry for the next + // call. if (operation_ != Operation::NONE && operation_ == transient_prev_op_ && offset_ == transient_prev_offset_ && @@ -947,7 +951,9 @@ void NomadNetCache::service() { write_open_ == transient_prev_write_open_) { if (transient_stall_count_ < MAX_TRANSIENT_STALL_TICKS) { ++transient_stall_count_; - } else { + if (transient_stall_count_ == 1) transient_stall_start_ms_ = now_ms; + } else if (now_ms >= transient_stall_start_ms_ && + now_ms - transient_stall_start_ms_ >= MAX_TRANSIENT_STALL_MS) { transient_stall_count_ = 0; transient_bail(); } @@ -1462,14 +1468,46 @@ void NomadNetCache::service() { void NomadNetCache::transient_bail() { // The storage seam has been stalled for far longer than any real SPI - // contention or SD mount window. Stop retrying: drop all namespace - // authority (lookups and commits now bypass), and clear the in-flight op - // so the caller's flow falls through to a live fetch, restoring the - // pre-cache page-load behavior instead of a frozen UI. + // contention or SD mount window. Stop retrying: release any in-flight + // storage resources (read handle, partial write, directory enumeration) + // so nothing leaks, drop all namespace authority (lookups and commits now + // bypass), and clear the in-flight op so the caller's flow falls through + // to a live fetch, restoring the pre-cache page-load behavior instead of + // a frozen UI. Each release is a bounded best effort: transient + // (BUSY/UNAVAILABLE) retries are re-tried a few times, and a still-open + // handle is accepted rather than re-pinning the op — the storage seam + // stays abandoned for the rest of the session, and its destructor closes + // whatever is left. transient_stall_count_ = 0; namespace_authoritative_ = false; - read_open_ = false; - write_open_ = false; + const bool list_was_active = + operation_ == Operation::RECOVERY_BEGIN || + operation_ == Operation::RECOVERY_NEXT || + operation_ == Operation::RECOVERY_END; + if (list_was_active) { + for (int attempt = 0; attempt < 3; ++attempt) { + const auto list_result = storage_.endList(); + if (!storage_result_is_transient(list_result)) break; + } + } + if (read_open_) { + for (int attempt = 0; attempt < 3; ++attempt) { + const auto read_result = storage_.endRead(); + if (!storage_result_is_transient(read_result)) { + read_open_ = false; + break; + } + } + } + if (write_open_) { + for (int attempt = 0; attempt < 3; ++attempt) { + const auto write_result = storage_.abortWrite(); + if (!storage_result_is_transient(write_result)) { + write_open_ = false; + break; + } + } + } commit_job_ = false; cleanup_stages_after_failure_ = false; eviction_pending_ = false; @@ -1481,7 +1519,10 @@ void NomadNetCache::transient_bail() { metadata_bytes_.clear(); ExternalVector().swap(body_); operation_ = Operation::NONE; - result_ = CacheResult::BYPASS; + // MISS (not BYPASS): for an admitted reload invalidation the flow accepts + // MISS as "the stale generation will not be served", which correctly falls + // through to a live fetch; a failed bail must never report a hard error. + result_ = CacheResult::MISS; } void NomadNetCache::cancel() { diff --git a/lib/tdeck_ui/UI/LXMF/NomadNetCache.h b/lib/tdeck_ui/UI/LXMF/NomadNetCache.h index c3d7f46c..6adc3d86 100644 --- a/lib/tdeck_ui/UI/LXMF/NomadNetCache.h +++ b/lib/tdeck_ui/UI/LXMF/NomadNetCache.h @@ -93,7 +93,11 @@ public: CacheResult beginRecovery(std::uint64_t now, bool cleanup_stages = false); CacheResult invalidate(const CacheKey& key); - void service(); + // now_ms is a monotonic millisecond clock used to bound the transient-stall + // bail in wall-time (see the stall-guard note below). Production passes + // millis(); 0 is a safe default for tests that never expect a stall (the + // time window is only reached after 500 consecutive no-progress ticks). + void service(std::uint64_t now_ms = 0); void cancel(); bool busy() const { return operation_ != Operation::NONE; } bool recoveryComplete() const { return recovery_complete_; } @@ -234,12 +238,21 @@ private: // offset, scan/cleanup index, scan count, open-flag) resets the stall // counter, so slow-but-progressing steps (chunked reads/writes, directory // scans) never false-trip; a tick with no advance is a transient stall. - // Past the bounded budget the cache bails: mark the namespace - // non-authoritative for the session (lookups/commits then bypass) and - // clear the op, so the flow falls through to a live fetch (the pre-cache - // behavior). 500 no-progress ticks far exceeds any real SPI contention or - // SD mount window (each op already waits only 100 ms on the bus mutex). + // + // The bail is time-bounded, not tick-bounded: a not-ready SD seam returns + // UNAVAILABLE immediately (no bus wait), so the main loop can spin many + // ticks in a second and a pure tick budget would expire during a + // legitimate mount window and disable caching for the whole session. + // Both a tick floor (protects a pathological fast tick loop) AND an elapsed + // wall-time window must pass before the cache bails. 10s comfortably + // exceeds the one-time boot mount window (~2.5s, and recovery starts after + // UI-ready with the card already mounted) and any SPI contention burst. + // On bail the namespace is marked non-authoritative for the session + // (lookups/commits then bypass) and the in-flight op is cleared, so the + // flow falls through to a live fetch (the pre-cache behavior) instead of + // a frozen UI. static constexpr std::uint32_t MAX_TRANSIENT_STALL_TICKS = 500; + static constexpr std::uint64_t MAX_TRANSIENT_STALL_MS = 10000; Operation transient_prev_op_ = Operation::NONE; std::size_t transient_prev_offset_ = 0; std::size_t transient_prev_scan_index_ = 0; @@ -248,6 +261,7 @@ private: bool transient_prev_read_open_ = false; bool transient_prev_write_open_ = false; std::uint32_t transient_stall_count_ = 0; + std::uint64_t transient_stall_start_ms_ = 0; void transient_bail(); static constexpr std::size_t VERIFY_SCRATCH_BYTES = 1024; diff --git a/lib/tdeck_ui/UI/LXMF/NomadNetCacheFlow.cpp b/lib/tdeck_ui/UI/LXMF/NomadNetCacheFlow.cpp index 00a40140..8d822ac1 100644 --- a/lib/tdeck_ui/UI/LXMF/NomadNetCacheFlow.cpp +++ b/lib/tdeck_ui/UI/LXMF/NomadNetCacheFlow.cpp @@ -31,10 +31,10 @@ CacheFlowState NomadNetCacheFlow::begin(const CacheKey& key, std::uint64_t now, return state_; } -void NomadNetCacheFlow::service() { +void NomadNetCacheFlow::service(std::uint64_t now_ms) { if (state_ == CacheFlowState::CANCELLED || state_ == CacheFlowState::FAILED) return; - if (cache_.busy()) cache_.service(); + if (cache_.busy()) cache_.service(now_ms); if (state_ == CacheFlowState::INVALIDATE) { if (cache_.busy()) return; if (!invalidation_admitted_) { diff --git a/lib/tdeck_ui/UI/LXMF/NomadNetCacheFlow.h b/lib/tdeck_ui/UI/LXMF/NomadNetCacheFlow.h index 90c171aa..a4baec15 100644 --- a/lib/tdeck_ui/UI/LXMF/NomadNetCacheFlow.h +++ b/lib/tdeck_ui/UI/LXMF/NomadNetCacheFlow.h @@ -22,7 +22,7 @@ public: explicit NomadNetCacheFlow(NomadNetCache& cache) : cache_(cache) {} CacheFlowState begin(const CacheKey&, std::uint64_t now, bool reload); - void service(); + void service(std::uint64_t now_ms = 0); bool acceptLive(const std::vector&, const CacheEligibility&, std::uint64_t now); void cancel(); diff --git a/lib/tdeck_ui/UI/LXMF/UIManager.cpp b/lib/tdeck_ui/UI/LXMF/UIManager.cpp index e7d44467..1350ba5b 100644 --- a/lib/tdeck_ui/UI/LXMF/UIManager.cpp +++ b/lib/tdeck_ui/UI/LXMF/UIManager.cpp @@ -3101,7 +3101,7 @@ void UIManager::nomad_update() { _nomad_cache_pending_now = 0; _nomad_cache_pending_ttl = 0; } - _nomad_cache_flow.service(); + _nomad_cache_flow.service(millis()); if (_nomad_state == NomadState::CACHE) { if (_nomad_cache_generation != _nomad_navigation_generation) { _nomad_cache_flow.cancel(); diff --git a/tests/native/test_nomadnet_cache.cpp b/tests/native/test_nomadnet_cache.cpp index 24e58729..db240f6b 100644 --- a/tests/native/test_nomadnet_cache.cpp +++ b/tests/native/test_nomadnet_cache.cpp @@ -23,7 +23,7 @@ struct MemoryStorage final:NomadNetStorage{ StorageResult stat(const char*n,uint32_t&s)override{++operations;if(n==stat_fail_path&&stat_fail_result!=StorageResult::OK)return stat_fail_result;auto i=files.find(n);if(i==files.end())return StorageResult::MISS;s=i->second.size();return StorageResult::OK;} StorageResult beginList(const char*d)override{++operations;list.clear();for(auto&f:files)if(f.first.rfind(std::string(d)+"/",0)==0)list.push_back(f.first);li=0;return available?list_result:StorageResult::UNAVAILABLE;} StorageResult nextList(char*n,size_t c,bool&done)override{++operations;if(next_list_fail_at&&static_cast(li+1)==next_list_fail_at)return StorageResult::IO_ERROR;if(li==list.size()){done=true;return StorageResult::OK;}done=false;if(list[li].size()+1>c){++li;return StorageResult::TOO_LARGE;}std::memcpy(n,list[li].c_str(),list[li].size()+1);++li;return StorageResult::OK;} - StorageResult endList()override{++operations;return StorageResult::OK;}std::vectorlist;size_t li=0; + StorageResult endList()override{++operations;if(list_close_busy)return StorageResult::BUSY;return StorageResult::OK;}std::vectorlist;size_t li=0;bool list_close_busy=false; }; static CacheKey key(const char*path="/page/index.mu"){return CacheKey{"0123456789abcdef0123456789abcdef",path,RequestDataClass::NIL};} // A seam whose directory enumeration is permanently BUSY (SPI mutex starved @@ -68,12 +68,43 @@ int main(){int f=0;auto ck=[&](bool x,const char*n){if(!x){++f;std::cerr<<"FAIL // A persistently transient seam during boot-time recovery must not pin the // cache (and the UI). Before this fix the recovery retried beginList forever; // now the stall budget expires, the namespace is marked non-authoritative, - // and the op clears so the flow can fall through to a live fetch. + // and the op clears so the flow can fall through to a live fetch. The bail is + // gated on wall-time, so the driver advances a fake monotonic clock (1ms per + // tick) so the time window elapses in a bounded number of ticks. + auto stall_run=[](NomadNetCache&c){uint64_t now=0;for(int i=0;i<200000&&c.busy();++i){now+=1;c.service(now);}}; MemoryStorage busy_boot;busy_boot.available=false;NomadNetCache bc(busy_boot,cfg); - for(int i=0;i<2000&&bc.busy();++i)bc.service(); + stall_run(bc); ck(!bc.busy(),"persistent transient recovery bails instead of pinning"); ck(!bc.recoveryComplete(),"bailed recovery is not authoritative"); ck(bc.beginLookup(key(),100)==CacheResult::BYPASS,"post-bail lookup bypasses to live"); + ck(busy_boot.active.empty(),"bail does not leak a read handle"); + // F3: 500 fast no-progress ticks are NOT enough to bail; the wall-time window + // (10s) must also elapse. With a flat (slow) clock the stall must survive well + // past the tick floor so a healthy SD that stays briefly quiet keeps its cache + // authority for the session. + MemoryStorage slow_boot;slow_boot.available=false;NomadNetCache sc(slow_boot,cfg); + uint64_t flat=0;for(int i=0;i<600&&sc.busy();++i){flat+=1;sc.service(flat);} + ck(sc.busy(),"fast ticks alone do not disable the session cache"); + // A seam that heals within the window terminates recovery cleanly. Boot + // recovery transients do not degrade the namespace (the RECOVERY_BEGIN/END + // paths keep it authoritative), so a healable transient still completes + // recovery authoritatively and the session cache remains usable. + MemoryStorage recover;recover.available=false;NomadNetCache hr(recover,cfg); + uint64_t hrnow=0;int guard=0;for(;hr.busy()&&guard<100000;++guard,++hrnow){if(guard==500)recover.available=true;hr.service(hrnow);} + ck(!hr.busy(),"seam that heals within the window terminates recovery"); + ck(hr.recoveryComplete(),"healable transient keeps the namespace authoritative"); + ck(hr.beginLookup(key(),100)==CacheResult::PENDING,"post-heal lookup uses the cache, not a bypass"); + // F2: a bail while the directory enumeration handle is open (RECOVERY_END + // endList stuck BUSY) must release it rather than leak it. A fresh boot + // recovery opens the list in RECOVERY_BEGIN and closes it in RECOVERY_END, so + // a seam whose endList is persistently BUSY reaches the guard with the list + // handle open — exactly the device case a stale SPI mutex can produce. + MemoryStorage list_seam;list_seam.list_close_busy=true; + NomadNetCache list_recover(list_seam,cfg); + uint64_t lnow=0;for(int i=0;i<200000&&list_recover.busy();++i){lnow+=1;list_recover.service(lnow);} + ck(!list_recover.busy(),"list-open stall bails instead of pinning"); + ck(!list_recover.recoveryComplete(),"bailed recovery is not authoritative"); + ck(list_recover.beginLookup(key(),100)==CacheResult::BYPASS,"post list-bail lookup bypasses to live"); // Deterministic expired-first then oldest quota eviction. MemoryStorage s3;NomadNetCache c3(s3,cfg);drain(c3,&s3);for(int i=0;i<3;i++){auto k=key((std::string("/page/")+char('a'+i)).c_str());ck(c3.beginCommit(k,body,400+i,i==0?1:100)==CacheResult::PENDING,"quota commit");drain(c3);}ck(c3.entryCount()<=2&&c3.totalBytes()<=cfg.max_bytes,"quotas bounded");ck(c3.beginLookup(key("/page/a"),500)==CacheResult::PENDING,"evicted lookup");drain(c3);ck(c3.lastResult()!=CacheResult::HIT,"expired evicted first"); diff --git a/tests/native/test_nomadnet_cache_flow.cpp b/tests/native/test_nomadnet_cache_flow.cpp index 27d4d92f..17aaa66c 100644 --- a/tests/native/test_nomadnet_cache_flow.cpp +++ b/tests/native/test_nomadnet_cache_flow.cpp @@ -36,9 +36,30 @@ int main(){int f=0;auto ck=[&](bool x,const char*n){if(!x){f++;std::cerr<<"FAIL Mem hang;hang.list_busy=true;NomadNetCache hc(hang);NomadNetCacheFlow hf(hc); CacheKey hk{"fedcba9876543210fedcba9876543210","/page/hang.mu",RequestDataClass::NIL}; ck(hf.begin(hk,1000,false)==CacheFlowState::LOOKUP,"hang lookup admitted"); - int serviced=0; - while(hf.state()==CacheFlowState::LOOKUP&&serviced<10000){hf.service();++serviced;} + uint64_t hclock=0;int serviced=0; + while(hf.state()==CacheFlowState::LOOKUP&&serviced<200000){hf.service(++hclock);++serviced;} ck(hf.state()==CacheFlowState::NEED_LIVE,"pinned recovery no longer freezes the lookup"); - ck(serviced<10000,"lookup reached live in bounded service ticks"); + ck(serviced<200000,"lookup reached live in bounded service ticks"); + ck(hc.recoveryComplete()==false,"bailed recovery is not authoritative"); + // F3: a flat (non-advancing) clock must NOT bail — the wall-time window has to + // elapse, so 500 fast no-progress ticks alone keep the cache (session) alive. + { + Mem hangf;hangf.list_busy=true;NomadNetCache hcf(hangf);NomadNetCacheFlow hff(hcf); + CacheKey hkf{"fedcba9876543210fedcba9876543210","/page/hangf.mu",RequestDataClass::NIL}; + hff.begin(hkf,1000,false); + uint64_t flat=0;for(int i=0;i<600&&hff.state()==CacheFlowState::LOOKUP;++i){hff.service(flat);++flat;} + ck(hff.state()==CacheFlowState::LOOKUP,"flat clock keeps a fast-ticking cache from bailing"); + } + // F1: a bail during an admitted RELOAD invalidation must fall through to a + // live fetch (NEED_LIVE), not report a hard "Page cache invalidation failed". + { + Mem hangr;hangr.list_busy=true;NomadNetCache hcr(hangr);NomadNetCacheFlow hfr(hcr); + CacheKey hkr{"fedcba9876543210fedcba9876543210","/page/hangr.mu",RequestDataClass::NIL}; + hfr.begin(hkr,1000,true); + uint64_t rclock=0;int svc=0; + while((hfr.state()==CacheFlowState::INVALIDATE)&&svc<200000){hfr.service(++rclock);++svc;} + ck(hfr.state()==CacheFlowState::NEED_LIVE,"reload invalidation bail falls through to live"); + ck(hfr.state()!=CacheFlowState::FAILED,"reload invalidation bail is not a hard failure"); + } } std::cout<<(f?"failed":"passed")<<"\n";return f?1:0;} From fabd3713df0f6696258ffcee39b9ea7d346cc87a Mon Sep 17 00:00:00 2001 From: Torlando <281092095+torlando-agent[bot]@users.noreply.github.com> Date: Tue, 15 Sep 2026 14:03:25 +0000 Subject: [PATCH 3/3] fix(nomadnet): make the transient-stall time window wrap-safe across millis() A stall spanning the 32-bit millis() rollover zero-extends to a value smaller than the pre-wrap start, so the now_ms >= start_ms guard blocked the bail until the counter lapped the ~49.7-day start value. Use 32-bit unsigned subtraction, which measures true elapsed time across the wrap. Regression: a stall starting near the counter max and wrapping bails in bounded ticks. --- lib/tdeck_ui/UI/LXMF/NomadNetCache.cpp | 7 +++++-- tests/native/test_nomadnet_cache.cpp | 13 +++++++++++++ 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/lib/tdeck_ui/UI/LXMF/NomadNetCache.cpp b/lib/tdeck_ui/UI/LXMF/NomadNetCache.cpp index 6b9ea245..249ad640 100644 --- a/lib/tdeck_ui/UI/LXMF/NomadNetCache.cpp +++ b/lib/tdeck_ui/UI/LXMF/NomadNetCache.cpp @@ -952,8 +952,11 @@ void NomadNetCache::service(std::uint64_t now_ms) { if (transient_stall_count_ < MAX_TRANSIENT_STALL_TICKS) { ++transient_stall_count_; if (transient_stall_count_ == 1) transient_stall_start_ms_ = now_ms; - } else if (now_ms >= transient_stall_start_ms_ && - now_ms - transient_stall_start_ms_ >= MAX_TRANSIENT_STALL_MS) { + } else if (static_cast(now_ms - transient_stall_start_ms_) >= + static_cast(MAX_TRANSIENT_STALL_MS)) { + // 32-bit unsigned subtraction is wrap-safe across the millis() + // wraparound, so the 10s window measures true elapsed time even + // when the stall spans the 49.7-day counter rollover. transient_stall_count_ = 0; transient_bail(); } diff --git a/tests/native/test_nomadnet_cache.cpp b/tests/native/test_nomadnet_cache.cpp index db240f6b..9684be02 100644 --- a/tests/native/test_nomadnet_cache.cpp +++ b/tests/native/test_nomadnet_cache.cpp @@ -85,6 +85,19 @@ int main(){int f=0;auto ck=[&](bool x,const char*n){if(!x){++f;std::cerr<<"FAIL MemoryStorage slow_boot;slow_boot.available=false;NomadNetCache sc(slow_boot,cfg); uint64_t flat=0;for(int i=0;i<600&&sc.busy();++i){flat+=1;sc.service(flat);} ck(sc.busy(),"fast ticks alone do not disable the session cache"); + // F3/wrap: the wall-time window must measure true elapsed time across the + // 32-bit millis() rollover. millis() is a 32-bit counter zero-extended at the + // call site, so model it that way: a stall that starts 7.8s below the counter + // max and wraps must still bail after 10s of elapsed ticks, not wait ~49.7 + // days for the counter to lap the pre-wrap start value. + MemoryStorage wrap_boot;wrap_boot.available=false;NomadNetCache wc(wrap_boot,cfg); + const uint64_t wrap_pre=4294959500ULL; + for(int i=0;i<500&&wc.busy();++i)wc.service(wrap_pre+i); + ck(wc.busy(),"stall still active at tick floor before wrap"); + uint64_t now=4294960000ULL;int wcap=0; + while(wc.busy()&&wcap<200000){wc.service(static_cast(now));++now;++wcap;} + ck(!wc.busy(),"stall spanning the millis wrap still bails"); + ck(wcap<200000,"wrap-spanning stall bails in bounded ticks, not ~49.7 days"); // A seam that heals within the window terminates recovery cleanly. Boot // recovery transients do not degrade the namespace (the RECOVERY_BEGIN/END // paths keep it authoritative), so a healable transient still completes