Skip to content

fix(cache): treat a cache entry with an overflowing duration as corrupt - #786

Merged
wan9chi merged 3 commits into
mainfrom
claude/issue-780-pr-317381
Oct 2, 2026
Merged

wan9chi merged 3 commits into
mainfrom
claude/issue-780-pr-317381

Conversation

@wan9chi

@wan9chi wan9chi commented Oct 2, 2026 •

Copy link
Copy Markdown
Member

Motivation

Fixes #780. A cache entry stores the task's duration as seconds plus nanoseconds, and decoding it called Duration::new without checking them. Seconds near u64::MAX with nanoseconds that carry over panic there, and release builds abort on panic, so a single corrupt or malicious remote cache entry killed the whole vp run instead of being a cache miss.

Changes

  • Remove the hand-written DurationSchema and use wincode's built-in Duration schema. The adapter dates from the bincode migration (refactor: migrate from bincode to wincode #334), when wincode 0.5 had no Duration support. wincode 0.6's schema has the same encoding (u64 seconds, then u32 nanoseconds) and returns a decode error on overflow. The on-disk and remote formats don't change, so the cache schema version stays the same.
  • Such an entry now fails to decode like any other corrupt remote value: it's a cache miss with remote cache value is corrupt as its reason.

🤖 Generated with Claude Code

wan9chi and others added 2 commits October 2, 2026 10:02
Decoding a cache entry built its duration with `Duration::new` without
checking the fields, so seconds near `u64::MAX` plus nanoseconds that
carry over panicked and aborted the run. Use wincode's own `Duration`
schema, which has the same encoding and rejects the overflow, so a
remote entry like this is a corrupt value and a cache miss.

Fixes #780

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions

github-actions Bot commented Oct 2, 2026 •

Copy link
Copy Markdown

fspy benchmark

linux

dynamic/launch             change  -0.16%  [ -6.12% ..  +5.31%]  overhead  +295.41%
dynamic/access             change  +0.00%  [ -1.50% ..  +1.05%]  overhead   +14.60%
dynamic/access-relative    change  -0.32%  [ -1.19% ..  +0.76%]  overhead   +59.18%
dynamic/access-contended   change  +0.27%  [ -3.05% ..  +5.93%]  overhead   +17.46%
static/launch              change  -0.48%  [ -5.52% ..  +5.97%]  overhead  +750.21%
static/access              change  +0.16%  [ -2.32% ..  +1.26%]  overhead  +807.46%
static/access-relative     change  +0.09%  [ -2.29% ..  +1.37%]  overhead +1373.91%
static/access-contended    change  +0.03%  [ -1.36% ..  +2.10%]  overhead +3145.10%

macos

dynamic/launch             change  +0.46%  [ -7.02% .. +10.16%]  overhead  +221.76%
dynamic/access             change  +0.00%  [-11.06% .. +53.53%]  overhead    +1.30%
dynamic/access-relative    change  +0.76%  [-18.66% .. +51.04%]  overhead  +268.98%
dynamic/access-contended   change  -0.01%  [-58.74% .. +63.58%]  overhead    -0.78%

windows

dynamic/launch             change  -0.04%  [ -1.58% ..  +1.09%]  overhead   +25.61%
dynamic/access             change  +0.19%  [ -0.58% ..  +0.78%]  overhead    +1.16%
dynamic/access-relative    change  +0.18%  [ -1.08% ..  +1.30%]  overhead    +1.12%
dynamic/access-contended   change  +0.30%  [ -5.31% .. +12.87%]  overhead    +1.10%

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@wan9chi
wan9chi merged commit 8630454 into main Oct 2, 2026
19 checks passed
@wan9chi
wan9chi deleted the claude/issue-780-pr-317381 branch October 2, 2026 02:12
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.

A corrupt remote cache entry can crash vp run

1 participant