Skip to content

fix(storage): release recovery writer locks on error exits (#1283) - #1284

Merged
DecisionNerd merged 1 commit into
mainfrom
fix/1283-retention-writer-lock
Sep 15, 2026
Merged

DecisionNerd merged 1 commit into
mainfrom
fix/1283-retention-writer-lock

Conversation

@DecisionNerd

@DecisionNerd DecisionNerd commented Sep 15, 2026 •

Copy link
Copy Markdown
Contributor

A cleanup resource-limit error dropped the parent writer-lock file without explicitly unlocking it. A duplicated or fork-inherited descriptor could therefore keep the kernel lock alive and make the next writer receive GF_WRITER_BUSY.

Return a private writer-lock guard from recovery-lock acquisition. Its error-path destructor explicitly unlocks; successful paths retain fallible explicit release and close the file before project-root deletion. All four retention/recovery callers preserve checkpoint-before-writer release order and allocation accounting.

Two deterministic regressions keep a duplicate of the production writer descriptor alive across max_entries and max_bytes_scanned errors. Both failed on the prior implementation with GF_WRITER_BUSY. They require independent writer acquisition and a repeated public cleanup preview while the duplicate remains open. Existing genuine-contention coverage remains enabled. This proves the ownership defect; fork inheritance as the trigger of the earlier CI occurrence remains an inference.

Validation: cargo test --locked --release -p graphforge-storage project_retention::tests -- --test-threads=4 passed all 15 tests. The same optimized storage test executable passed all 29 recovery tests with four test threads. The pre-fix duplicated_writer_lock filter failed both tests as expected. Workspace Clippy, formatting and make pre-push-fast pass; independent review found no actionable findings. No public API, durable-format, RSS-policy, or scale-workload change.

Closes #1283. Prerequisite for completing #1278 via PR #1281.

@coderabbitai

coderabbitai Bot commented Sep 15, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Advanced

Run ID: 8fca2275-9e82-4abb-bdcd-2cffa28d650f

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

Warning

Billing warning: we have not been able to collect payment for this subscription for more than 72 hours. Please update the payment method or pay any pending invoices in Billing to avoid service interruption.


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.

@github-actions github-actions Bot added the core Core source code changes label Sep 15, 2026
@DecisionNerd
DecisionNerd merged commit 233dddd into main Sep 15, 2026
21 checks passed
@DecisionNerd
DecisionNerd deleted the fix/1283-retention-writer-lock branch September 15, 2026 02:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

core Core source code changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix(storage): release retention writer locks on bounded-error exits

1 participant