Repository navigation
fix(descriptor_import): revalidate cleanup victims under the identity lock (0.25.1) - #180
Merged
Merged
Conversation
… lock (0.25.1) cleanup() scanned the root without any identity lock and later deleted a victim by pathname under the lock, trusting the stale scan record. With several processes sharing a cache: A scans old artifact X; B evicts X; C re-materializes the same identity at the same path and releases its lease; A takes X's now-free lock and deletes C's new artifact. Eviction now re-checks, under the exclusive lock, that the victim is still a regular file with the scanned size and the scanned LRU stamp (the .touch sidecar's mtime, artifact mtime as fallback); anything that changed or vanished is skipped without reclaim credit and without counting as an error. Orphaned partials are re-checked for existence and age the same way. testing::cleanupWithProbe() runs a hook between the scan and the eviction so the interleaving is deterministic in tests. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Y1RDvhAG9qkeQNVjkK2wut
facontidavide
added a commit
that referenced
this pull request
Aug 31, 2026
0.25.0 was never published before the descriptor-import cleanup fix (#180) landed, so the fix shipped in the v0.25.0 release; the CHANGELOG still listed it under a separate [0.25.1] heading. Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Patch release 0.25.1:
RequestArtifactCache::cleanup()revalidates each eviction victim under its exclusive identity lock before deleting it.Why
Found by an adversarial review of the first consumer (pj-official-plugins #275).
cleanup()scans the root without holding any identity lock, then acquires the lock per victim and removes the pathname using the stale scan record. With several PlotJuggler instances sharing a cache:Fix
Under the exclusive lock the victim must still be a regular file with the scanned size and the scanned LRU stamp (
.touchsidecar mtime, artifact mtime as fallback — a victim scanned without a readable stamp can never match). Anything changed or vanished is skipped: no reclaim credit, nohad_errors(benign contention). Orphaned partials are re-checked for existence and age under the lock the same way.testing::cleanupWithProbe()(pj_base/sdk/testing/request_cache_probe.hpp, same pattern as the provider-job gate probe) runs a hook between the scan and the eviction so the interleaving is deterministic in tests: republish-between-scan-and-evict is not deleted, an unchanged victim still is, and a refreshed partial survives.No C-ABI change (
PJ_ABI_VERSION/abi/baseline.abiuntouched). VERSION 0.25.1, CHANGELOG entry.Verification
Release 83/83, ASAN 83/83, pre-commit clean; edited TUs confirmed recompiled.
🤖 Generated with Claude Code
https://claude.ai/code/session_01Y1RDvhAG9qkeQNVjkK2wut