fix(aich): true linear-scan fallback when LoadHashSet cache offset is stale - #191
Merged
got3nks merged 1 commit intoJun 17, 2026
Merged
Conversation
… stale PR amule-project#186 added an offset cache so LoadHashSet seeks straight to the matching entry instead of linearly walking known2.met. The previous comment claimed that on a first-read mismatch we'd "fall through to the linear scan from the start as defensive recovery" — but the loop actually keeps reading from the cached offset onward, treating whatever bytes are at that position as a root hash + hashCount. If the cache is stale (known2.met modified externally between cache load and the request) the offset can land in the middle of a hash blob; we'd misread CurrentHash, misread nHashCount, and the loop would misalign and ultimately return false even though the entry still exists somewhere in the file. Reported by @danim7 in amule-org#186. Add a real defensive rewind: on the first iteration after seeking to the cached offset, if CurrentHash doesn't match our root hash, treat the cache as stale, jump back to just past the version header (byte 1), and continue with a true linear scan from the top. Cap the rewind to one attempt with cacheFallbackTriggered so we can't loop. Also handle the corner case where the cached offset is past EOF (file was truncated externally): skip the seek entirely and fall straight through to the linear scan. In the happy path (cache offset correct), behavior is unchanged: the first read matches and we return true after the existing match handler.
This was referenced Jun 17, 2026
got3nks
added a commit
that referenced
this pull request
Jun 17, 2026
PR #191 added a defensive rewind so LoadHashSet finds the right entry even when the cached offset is stale (known2.met modified externally between cache load and the request). The rewind worked but didn't update the cache: every subsequent LoadHashSet for the same stale root hash kept paying the seek-miss + full linear scan, instead of the O(1) cache hit the cache exists to provide. The cache only self-cleared on daemon restart, on CEOFException-driven truncation, or after an orphan-prune rewrite in CAICHSyncTask -- none of which fire just because a single entry's offset went stale. When the rewind succeeds, the linear scan has just found the right position for the root hash. Stamp it back into s_rootHashCache so future lookups for the same hash go straight there. Capture the entry's start position pre-read (entryStartPos) so we have the exact offset of the root hash, matching the value LoadRootHashCacheLocked and SaveHashSet write into the cache.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Follow-up to #186, addressing point 1 of @danim7's post-merge review.
PR #186 added an offset cache so
LoadHashSetseeks straight to the matching entry inknown2.metinstead of walking it linearly. The previous inline comment claimed that on a first-read mismatch we'd "fall through to the linear scan from the start as defensive recovery". @danim7 pointed out — correctly — that the loop actually keeps reading from the cached offset onward and just skipsnHashCount * HASHSIZEbytes on each mismatch. If the cache is stale (known2.metmodified externally between cache load and request), the offset could point into the middle of a hash blob; we'd misreadCurrentHash, misreadnHashCount, and the loop would walk off into garbage and ultimately returnfalseeven though the entry still exists somewhere in the file.What this changes
In
LoadHashSet:CurrentHashdoesn't matchm_pHashTree.m_Hash, treat the cache as stale, rewind to byte 1 (just past the version header), and continue with a true linear scan from the top. Guarded bycacheFallbackTriggeredso the rewind can fire at most once per call.>= file.GetLength()(file was truncated externally), skip the seek entirely and fall straight to the linear scan.In the happy path (cache correct), behaviour is unchanged: the first read matches and the existing match handler runs.
Verification
verify-pr186-aich.pystill passes (single-file happy path).verify-loadhashset-multi-file.py(inscripts/, not part of this PR): boots amuled with two shared files of different sizes, waits for both AICH trees to land inknown2_64.met, then sends two OP_AICHREQUESTs (one per file) on a single connection within the rate-limit budget. Both files resolve to non-emptyOP_AICHANSWERpayloads (152 B for file A, 306 B for file B), confirming the cache correctly returns distinct offsets per entry.The stale-cache rewind path itself is not directly exercised by these tests — reaching it requires external modification of
known2.metbetween cache load and request, which isn't reachable from a black-box harness without an instrumentation hook. The code path is small (~20 lines) and reviewable on inspection.Test plan