[zmanian]

Summary

Fixes #495.

The heartbeat system spawns run_if_due() via tokio::spawn on every tick (default 30 min), creating a TOCTOU race where multiple tasks read the state file before any saves it, causing all to execute concurrently. On Windows this also triggers OS error 1224 when multiple tasks write the same file simultaneously.

Three fixes:

• AtomicBool guard (RUNNING + RunningGuard RAII) ensures only one hygiene pass runs at a time
• State saved before cleanup (not after) to claim the cadence window early and close the TOCTOU race window
• Atomic file write (write to .json.tmp, then rename) avoids Windows file-locking errors from concurrent writers

Test plan

• cargo test hygiene -- 8 passed (including 2 new regression tests)
• cargo clippy --all --all-features -- zero warnings
• Manual: verify on Windows that the "os error 1224" no longer appears during AFK periods

Generated with Claude Code

[gemini-code-assist]

Summary of Changes

Hello, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request addresses issues related to concurrent execution of memory hygiene passes, specifically preventing race conditions and resolving Windows file-locking errors. The changes ensure that hygiene operations are serialized and state updates are handled robustly, improving the reliability and stability of the system's cleanup routines.

Highlights

• Concurrency Control: Implemented an AtomicBool guard (RUNNING with RunningGuard RAII) to ensure that only one memory hygiene pass executes at a time, preventing race conditions.
• Cadence Window Management: Modified the hygiene process to save the state before performing cleanup. This claims the cadence window earlier, reducing the time frame for Time-of-Check to Time-of-Use (TOCTOU) race conditions.
• Atomic File Writes: Refactored the state saving mechanism to use an atomic file write pattern (write to a .json.tmp file, then rename). This mitigates Windows file-locking errors (OS error 1224) that can occur with concurrent write attempts.

Changelog

• src/workspace/hygiene.rs
  • Updated module documentation to reflect the new concurrency guard and revised hygiene pass steps.
  • Added AtomicBool and Ordering imports for concurrency control.
  • Introduced a static AtomicBool named RUNNING to track if a hygiene pass is active.
  • Modified the run_if_due function to acquire the RUNNING guard, skipping execution if another pass is already in progress.
  • Created a RunningGuard RAII struct to automatically release the RUNNING guard when run_if_due exits.
  • Relocated the save_state call within run_if_due to occur before the cleanup logic.
  • Refactored the save_state function to implement an atomic write strategy using a temporary file and std::fs::rename.
  • Enhanced error handling and logging within save_state for directory creation and file operations.
  • Added two new test cases: one to verify the atomic nature of save_state and another to confirm the RUNNING guard prevents re-entry.

Activity

• The author has provided a detailed summary of the changes and the problem being solved.
• A test plan is outlined, including unit tests (cargo test hygiene), static analysis (cargo clippy), and a manual verification step for Windows.
• Unit tests for hygiene passed (8 tests, including 2 new regression tests).
• Static analysis with cargo clippy yielded zero warnings.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	/gemini review	Performs a code review for the current pull request in its current state.
Pull Request Summary	/gemini summary	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	/gemini help	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

[gemini-code-assist]

Code Review

This pull request effectively addresses a concurrency issue in the memory hygiene system. The introduction of an AtomicBool with an RAII guard is a robust solution to prevent concurrent hygiene passes, and the switch to an atomic file write pattern correctly resolves potential file locking errors on Windows. The logic is sound, the changes are well-documented, and the new regression tests are a valuable addition that verifies the fixes. I have one minor suggestion to improve the new test's robustness against parallel execution.

[gemini-code-assist]

This test modifies the global static RUNNING. Since cargo test runs tests in parallel by default, this could lead to flaky tests if other tests that also interact with the hygiene system are added in the future. To ensure this test is robust and doesn't interfere with other tests, it's best to serialize access to the shared global state. You can achieve this by adding a static Mutex at the tests module level and acquiring a lock at the beginning of this test.

[zmanian]

Addressed in 6a1a0a4. Added a static RUNNING_TESTS: Mutex<()> to the test module and the running_guard_prevents_reentry test now acquires it before touching the global RUNNING AtomicBool.