recall: with_open reuse guard misses store replacement, so a cached handle can point at a deleted inode
Nobody has claimed this yet.
- Dominant language
- Rust
- Stars
- 81.1k
- Forks
- 5.1k
- Avg merge
- 4d 21h
- Merged PRs (30d)
- 35
Description
src/core/retriever.rs caches one SQLite connection per thread and reuses it when the path is unchanged and still exists:
let reuse = matches!(&*cache, Some((p, _)) if *p == path && path.exists());
path.exists() catches deletion but not replacement. If the store is deleted and recreated (a manual rm then a later run, a restore from backup, a second checkout writing the same path), the cached handle still points at the old inode. Reads then resolve against a file nobody can see, so a hash printed in scrollback stops resolving with no error.
Suggested fix: compare device+inode (std::os::unix::fs::MetadataExt) rather than mere existence, or drop the cache when the file's identity changes.
Surfaced during review of #3278; not introduced by it.
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start in src/core/retriever.rs at the per-thread connection cache and its path.exists() reuse guard. Check how the cached handle is retained when the store is deleted and recreated, then use the issue's device-and-inode approach or drop the cache when file identity changes. Done means a recreated store does not reuse a handle for the deleted inode and subsequent reads resolve normally.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- rust, sqlite
- Domain
- cli, databases
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Active
- Clarity
- Mostly clear
- Newbie friendliness
- 72/100