Skip to content

fix: P9 β€” consolidate lock hoist, Condvar backpressure, TOCTOU hardening - #986

Merged
ajianaz merged 1 commit into
developfrom
fix/p9-backlog-n+1-lock-thread-safety
Aug 10, 2026
Merged

fix: P9 β€” consolidate lock hoist, Condvar backpressure, TOCTOU hardening#986
ajianaz merged 1 commit into
developfrom
fix/p9-backlog-n+1-lock-thread-safety

Conversation

@ajianaz

@ajianaz ajianaz commented Aug 10, 2026

Copy link
Copy Markdown
Collaborator

What

Performance and safety improvements addressing backlog items from cora re-scan analysis.

Why

These issues were identified during the P8 post-merge cora re-scan (163 findings) and represent the highest-impact backlog items:

  1. Lock contention in consolidate β€” write lock acquired + index saved per dedup pair (N iterations = N lock + N saves)
  2. CPU spin under load β€” server backpressure used yield_now() spin loop instead of parking threads
  3. TOCTOU race conditions β€” file existence check then open/create pattern allows race between processes
  4. Integer overflow risk β€” next_key + 1 could panic on u64 overflow (unlikely but undefined behavior)

Changes

consolidate.rs β€” Lock hoist + single save

  • Acquire index write lock once before the dedup loop
  • Single index.save() after all removals complete
  • Reduces N lock acquires + N disk saves β†’ 1 lock + 1 save

server/main.rs β€” Condvar backpressure

  • Replace atomic counter + spin-yield with (Mutex<usize>, Condvar) pair
  • Main accept loop parks when at capacity instead of burning CPU
  • Workers notify_one() on completion to wake the accept loop
  • Preserves spawn-error slot release semantics

vector.rs β€” TOCTOU + overflow hardening

  • load_or_create: OpenOptions::create_new(true) replaces if !exists() { write() } β€” atomic file creation eliminates race
  • Key mapping load: match read_to_string() replaces if exists() { read() } β€” eliminates check-then-read race
  • saturating_add(1) for next_key prevents u64 overflow panic at max value

Testing

  • Tests: 476 pass, 0 fail
  • Clippy: 0 warnings
  • Cora pre-commit: clean pass

…ing, saturating overflow

consolidate.rs:
- Hoist index write lock outside dedup loop β€” acquire once, save once
  instead of lock+save per pair (N pairs β†’ 1 lock + 1 save)

server/main.rs:
- Replace spin-yield backpressure with Condvar-based semaphore
- Thread park instead of CPU burn under load
- Preserve spawn-error slot release semantics

vector.rs:
- load_or_create: atomic file create via OpenOptions::create_new
  eliminates TOCTOU race (exists+write β†’ create_new)
- key mapping: replace exists()+read() with match on read_to_string
  (eliminates TOCTOU between check and read)
- saturating_add(1) for next_key to prevent u64 overflow panic

Test: 476 pass, 0 fail | Clippy: 0 warnings
let mut index = self
.index
.write()
.map_err(|_| Error::lock("index write lock during consolidate"))?;
@github-actions

Copy link
Copy Markdown

πŸ” Cora AI Code Review

❌ Blocked β€” critical issues found.

πŸ”΄ Error (1)

  • crates/uteke-core/src/consolidate.rs:258 β€” The write lock on self.index is now acquired once before the loop and held across all iterations. Inside the loop, self.delete_memory(...) is called, which likely acquires its own database lock (or the Uteke mutex). If any other code path acquires the DB lock first and then tries to read/write the index, this creates a lock-ordering inversion that can deadlock. Additionally, holding the write lock for the entire duration (including all SQLite operations) serializes and blocks all index access even during slow DB I/O, significantly reducing concurrency.

Review powered by cora-code Β· BYOK Β· MIT

@ajianaz
ajianaz merged commit 4074bcf into develop Aug 10, 2026
14 of 15 checks passed
@ajianaz
ajianaz deleted the fix/p9-backlog-n+1-lock-thread-safety branch August 10, 2026 03:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants