fix: dictionary lookup OOM on .dict.dz definitions

[W-Floyd]

Summary

• Goal: Fix .dict.dz dictionary lookups failing with "Not enough memory" (X3 dict lookup runs out of memory #2744). Two independent causes on the definition-extraction path, one commit each.

• Changes:

  1. Stop buffering the whole compressed chunk. extractChunkSlice() read the entire compressed chunk into one heap buffer before inflating it. A dictzip chunk is ~58 KB uncompressed — 18,428 bytes compressed for a real dictionary — and that block was held live while the 32 KB inflate window was allocated on top of it. Two large contiguous requests at once, on a heap sitting under 50 KB free mid-book. uzlib supports a pull callback, so the input is now fed from the file through a 2 KB refill buffer.

  2. Allocate largest-first. Even after (1), the lookup still failed — and the instrumented margin was twelve bytes:

  chunkSlice:enter          largest=36852   <- would have fit the ring
  after chunkSource (3348)  largest=32756
  inflateWindow need=32768  WILL FAIL
  

  Every buffer the lookup needs is carved from the same big free run, so allocation order decides the outcome. The ~3 KB ChunkSource was taken first, out of the only block large enough for the 32 KB ring. Reversing the order gives the ring the big block while it is still whole.

  	Before	After
  Largest block alongside the 32 KB ring	~18 KB (whole chunk)	~2.2 KB
  Ring allocated	after the small buffers	first

  Note the heap here is not badly fragmented — frag=15%, 14 free blocks. This was a shortfall-and-ordering bug, not a fragmentation one.

Scope Check

CrossPoint is intentionally narrow. See SCOPE.md and ROADMAP.md.
Please confirm:

• I have read SCOPE.md and ROADMAP.md.
• This PR is not a new built-in theme.
• This PR is not a new external network connector.
• This PR is not an interactive app, writing tool, RSS/news/browser, media playback, or PDF feature.
• No new feature surface at all — this is a bugfix to an already-in-scope feature (dictionary lookup). Stock reports this case as an error the user can do nothing about.
• This PR does not touch freeink-sdk/, lib/hal/, the bootloader, OTA, or recovery code, so no maintainer coordination is required on that basis.

Additional Context

• Behaviour is unchanged on success. A hit returns the same bytes; only the peak heap needed to produce them changed.
• The chunk boundary is still enforced. ChunkSource::remaining stops the callback at compressedSize, so it never reads into the next chunk — exactly the hard limit the whole-chunk buffer imposed.
• IO errors stay distinguishable from corruption. uzlib collapses a callback -1 into a generic decode failure, which would have turned every SD hiccup into Decompress and partly undone fix: name dictionary lookup failures (low-memory vs decompress vs read) instead of 'Not found' #2706. readFailed records the cause and maps it back to ReadError.
• InflateReader::initWithRing() is new: streaming mode over a caller-owned ring, needed because the reader is embedded in the struct whose allocation must come second. ownsRing keeps deinit() from freeing a buffer it doesn't own, so the existing init(true) callers in ZipFile and PngToBmpConverter are unaffected.
• Memory / flash impact: no new persistent RAM; peak transient demand per lookup drops by ~16 KB, and the 32 KB ring now comes out of the largest free block rather than whatever is left after it. ChunkSource (~2.2 KB, mostly the 2 KB refill buffer) is heap-allocated rather than stacked, to stay clear of the task stack budget.
• Reviewer focus: the uzlib callback contract in chunkReadCb (refill, set source/source_limit to the tail, return the first byte) and the window/src declaration order in extractChunkSlice — the ring must outlive the reader.
• Possible latency shift: SD access changes from one bulk chunk read to interleaved 2 KB reads. The discard phase stays pure sequential reads; only the extract phase (definition-sized, typically a few KB) alternates reads with writes to the temp file. No slowdown noticed in use, but it isn't benchmarked.
• Relationship to perf: open dictionary index files once per lookup #2733: disjoint files (that PR is Dictionary.{h,cpp}, the index-search path; this is DictZip.cpp + InflateReader). Neither depends on the other and they should not conflict. perf: open dictionary index files once per lookup #2733 does not fix this OOM — the failure is entirely downstream of what it changes.

Testing

• Builds clean for the device default env (ESP32-C3); clang-format clean.
• Host-verified against lib/uzlib with a real 58,315-byte chunk: full-chunk decode, mid-chunk slice (the case every lookup takes), and a slice ending exactly at the chunk boundary all match the source bytes; a truncated compressedSize is refused rather than over-read.
• Tested on hardware (Xteink X3): the lookup that reliably reproduced "Not enough memory" (focus reading + book style + anti-aliasing on) now succeeds, and no OOM has recurred in use since. This is hands-on use rather than a systematic soak, so I can't claim the failure mode is fully eliminated — but the specific reproducer is fixed.

---

AI Usage

Did you use AI tools to help write this code? YES — diagnosed with Claude Code using temporary heap instrumentation (largest-free-block probes, not included in this PR), and the fix was implemented with it. Reviewed and tested on hardware (X3) by me.

🤖 Generated with Claude Code

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 92302bab-9af0-4eb2-9f23-aebbd09a854f

📥 Commits

Reviewing files that changed from the base of the PR and between 98a0e5a and 353f84f.

📒 Files selected for processing (3)

• lib/InflateReader/InflateReader.cpp
• lib/InflateReader/InflateReader.h
• src/util/DictZip.cpp

🚧 Files skipped from review as they are similar to previous changes (2)

• lib/InflateReader/InflateReader.h
• src/util/DictZip.cpp

📜 Recent review details

⏰ Context from checks skipped due to timeout. (3)

• GitHub Check: cppcheck
• GitHub Check: unit-tests
• GitHub Check: build

🧰 Additional context used

🧠 Learnings (5)

📚 Learning: 2026-02-27T22:49:59.600Z

Learnt from: ngxson
Repo: crosspoint-reader/crosspoint-reader PR: 1218
File: src/activities/ActivityManager.cpp:254-265
Timestamp: 2026-02-27T22:49:59.600Z
Learning: In this codebase, assertions are always enabled (no NDEBUG). Use assert() to crash on programmer errors and surface logic bugs during development and in production builds. Do not rely on asserts for runtime error handling; they should enforce invariants that must always hold. Keep asserts side-effect free and inexpensive, and avoid relying on them for user-visible failures. Include <cassert> where appropriate and document the invariant being tested.

Applied to files:

• lib/InflateReader/InflateReader.cpp

📚 Learning: 2026-03-02T10:14:16.036Z

Learnt from: Uri-Tauber
Repo: crosspoint-reader/crosspoint-reader PR: 1245
File: lib/Epub/Epub/Section.cpp:277-308
Timestamp: 2026-03-02T10:14:16.036Z
Learning: Guideline: Strengthen serialization::readString to defend against unbounded growth when reading from disk data. Implement and enforce a maximum allowed length (e.g., a configured or reasonable constant) and validate the incoming length before resizing or allocating. Audit all call sites (e.g., BookMetadataCache, TextBlock, KOReaderCredentialStore, Section cache readers) to ensure they do not rely on unbounded len-based resizing. If the readString API must remain, add internal safeguards (bounds checks, length validation, and error handling) so per-call-site validations are not required. Ensure Section cache files remain versioned (SECTION_FILE_VERSION) and parameter mismatches invalidate caches, but do not rely on unsafe allocations; prefer safe, bounded reads with explicit errors on invalid data.

Applied to files:

• lib/InflateReader/InflateReader.cpp

📚 Learning: 2026-04-12T12:28:33.205Z

Learnt from: Uri-Tauber
Repo: crosspoint-reader/crosspoint-reader PR: 1629
File: src/activities/home/HomeActivity.cpp:119-120
Timestamp: 2026-04-12T12:28:33.205Z
Learning: When reviewing code in this repository (C/C++ sources), set review comment severity according to this policy:
- Use **Major** (🟠) only for defects with realistic risk of crash, out-of-memory (OOM), invalid pointer dereference, data corruption, or other severe issues that are unlikely to be caught in casual/manual device testing.
- Use **Minor** or informational for UX gaps, logic edge-cases, style issues, or missing feature completeness that the author can verify (or has verified) through normal device use.
- Do **not** escalate severity to Major based on behavioral/UX observations alone; assume the author has already tested the feature on their own device and only treat issues as Major if they match the high-risk defect categories above.

Applied to files:

• lib/InflateReader/InflateReader.cpp

📚 Learning: 2026-05-24T21:10:19.897Z

Learnt from: jeremydk
Repo: crosspoint-reader/crosspoint-reader PR: 2076
File: src/network/HttpDownloader.cpp:166-171
Timestamp: 2026-05-24T21:10:19.897Z
Learning: Follow the repo’s CLAUDE.md guidance: avoid adding error handling, fallbacks, or extra validation for scenarios that are provably unreachable given existing internal invariants and framework guarantees. Only add validation/guards at true system boundaries (e.g., user input, external APIs/network/IPC). For internal C++ APIs like HttpDownloader, do not add defensive checks against misuse (e.g., an empty std::function callback) when there is no reachable call site that can supply such values—guards in those cases are explicitly discouraged.

Applied to files:

• lib/InflateReader/InflateReader.cpp

📚 Learning: 2026-07-28T18:42:02.364Z

Learnt from: Uri-Tauber
Repo: crosspoint-reader/crosspoint-reader PR: 2772
File: lib/Epub/Epub/blocks/TextBlock.cpp:454-458
Timestamp: 2026-07-28T18:42:02.364Z
Learning: When deserializing untrusted data in C++ (e.g., via Serialization.h/serialization::readString), never read an untrusted uint32_t length and pass it directly to std::string::resize. Under -fno-exceptions, allocation failure can abort the process. Implement (and use) a centrally-available fallible, bounded readString API that validates the requested byte length before allocation—preferably by checking it against bytes remaining in the input/file. Migrate all call sites that deserialize strings (e.g., cache readers for metadata such as spine/TOC, image blocks, section anchor maps, and TextBlock ruby <rt> text) to use the new safe API. Do not introduce a fixed maximum “read-only” content cap unless the corresponding writer/parser also enforces the same constraint; otherwise current unbounded ruby <rt> content can cause cache rebuild loops due to mismatch between parse-time and read-time limits.

Applied to files:

• lib/InflateReader/InflateReader.cpp

🔇 Additional comments (1)

lib/InflateReader/InflateReader.cpp (1)

7-7: LGTM!

Also applies to: 30-37

---

📝 Walkthrough

Walkthrough

InflateReader now supports caller-owned ring buffers with ownership-aware cleanup. DictZip streams compressed chunks through a callback-backed input buffer instead of loading full chunks, while distinguishing underlying read failures from decompression failures.

Changes

Dictzip streaming decompression

Layer / File(s)	Summary
Ring buffer ownership contract lib/InflateReader/InflateReader.h, lib/InflateReader/InflateReader.cpp	Adds the shared ring-size constant, caller-owned ring initialization, and conditional ring-buffer cleanup.
Callback-backed chunk input src/util/DictZip.cpp	Adds ChunkSource and chunkReadCb to read compressed chunk data incrementally and track IO failures.
Streaming chunk extraction src/util/DictZip.cpp	Updates extractChunkSlice to use callback-backed decompression and classify read versus decompression errors.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
  participant DictZip
  participant HalFile
  participant chunkReadCb
  participant InflateReader

  DictZip->>HalFile: Seek to compressedOffset
  DictZip->>InflateReader: initWithRing(ring)
  InflateReader->>chunkReadCb: Request compressed bytes
  chunkReadCb->>HalFile: Read next input buffer
  HalFile-->>chunkReadCb: Bytes or IO failure
  chunkReadCb-->>InflateReader: Input data or end-of-chunk
  InflateReader-->>DictZip: Decompressed output or failure classification

Loading

Possibly related PRs

• crosspoint-reader/crosspoint-reader#2583: Added the dictzip range-extraction path that is refactored here.

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly summarizes the main bugfix: .dict.dz dictionary lookups hitting out-of-memory errors.
Description check	✅ Passed	The description is directly about the same dictzip memory fix and matches the implemented changes.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[W-Floyd]

Still doesn't (completely) fix the dictionary loading failures when under pressure/fragmentation.

[barbarhan]

I reviewed the new caller-owned ring-buffer path, ChunkSource callback streaming, allocation/destruction order, short-read handling, compressed-chunk boundary enforcement, and error classification.

The externally owned ring outlives InflateReader, ownsRing prevents both leaks and double-free, and every new DictZip read() call is made with a positive batch size. Short positive file reads are handled by refilling, while exhausted chunk input and actual I/O failures remain distinguishable.

CI is green, and I found no actionable issue introduced by this change.

[Uri-Tauber]

LGTM. Thanks @barbarhan