Skip to content

fix(storage): proactively flush expired memory entries#10

Merged
pi0 merged 1 commit intomainfrom
fix/cleanup
Mar 19, 2026
Merged

fix(storage): proactively flush expired memory entries#10
pi0 merged 1 commit intomainfrom
fix/cleanup

Conversation

@pi0
Copy link
Copy Markdown
Member

@pi0 pi0 commented Mar 19, 2026

Summary

  • Schedule a setTimeout per TTL entry to proactively delete it from the internal Map when it expires
  • Prevents unbounded memory growth from entries that are never read again (e.g. unique URLs with varying query params)
  • Timers use .unref() so they don't prevent process exit
  • Previous timers for the same key are cleared on overwrite or delete

related to nitrojs/nitro#2138

also fixed in unstorage: unjs/unstorage#759

Test plan

  • Added regression test: stores entries with short TTL, waits for expiry without reading, verifies proactive cleanup
  • All 87 existing tests pass
  • Typechecks clean

Summary by CodeRabbit

  • Bug Fixes

    • Improved in-memory storage TTL expiration handling to ensure expired entries are reliably cleaned up.
  • Tests

    • Added regression test to verify expired entries are proactively removed after TTL without explicit access.

schedule a `setTimeout` per TTL entry to delete it from the internal Map
when it expires, preventing unbounded memory growth from entries that are
never read again (e.g. unique URLs with query params).

closes nitrojs/nitro#2138
@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Mar 19, 2026

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: a2c8b55b-5fbe-47cd-9acd-37965491d46d

📥 Commits

Reviewing files that changed from the base of the PR and between 72250f4 and 1978ae4.

📒 Files selected for processing (2)
  • src/storage.ts
  • test/index.test.ts

📝 Walkthrough

Walkthrough

Enhanced in-memory storage with proactive TTL-based key expiration. A new timers map tracks timeout handles per key, enabling automatic deletion after TTL and cleanup on subsequent operations. Added a regression test verifying entries expire without explicit retrieval.

Changes

Cohort / File(s) Summary
TTL Expiration Implementation
src/storage.ts
Added timers map for per-key timeout management. set() now clears any existing timer before updating values. get() clears and removes expired timers. Timer cleanup centralized in _clearTimer() helper. Timers conditionally unref()'d for graceful process exit.
TTL Regression Test
test/index.test.ts
New test verifying proactive TTL-driven key expiration. Inserts entries with short TTL, waits for expiration, and confirms keys are removed (return null) without prior get() calls.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 Timers tick and hop away,
Keys that fade at end of day,
Cleanup happens, swift and clean,
No more stale cache in between!

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/cleanup
📝 Coding Plan
  • Generate coding plan for human review comments

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

Comment @coderabbitai help to get the list of available commands and usage tips.

@pi0 pi0 marked this pull request as ready for review March 19, 2026 13:37
@pi0 pi0 merged commit 153592e into main Mar 19, 2026
5 checks passed
@pi0 pi0 deleted the fix/cleanup branch March 19, 2026 13:37
pi0 added a commit to unjs/unstorage that referenced this pull request Mar 19, 2026
Schedule a setTimeout per TTL entry to delete it from the internal Map
when it expires, preventing unbounded memory growth from entries that
are never read again (e.g. unique URLs with varying query params).

Closes nitrojs/nitro#2138
Ref unjs/ocache#10
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.

1 participant