Skip to content

Fix non-atomic file writes that can corrupt cached data - #212

Open
h0bnobs wants to merge 1 commit into
mainfrom
fix/atomic-file-writes
Open

Fix non-atomic file writes that can corrupt cached data#212
h0bnobs wants to merge 1 commit into
mainfrom
fix/atomic-file-writes

Conversation

@h0bnobs

@h0bnobs h0bnobs commented Jun 10, 2026

Copy link
Copy Markdown
Collaborator

Description

Fixes #127.

Writes to the parquet price cache, stock_info.json and pickled model files were made directly to the final path. If the process crashed mid-write (OOM, SIGKILL, disk full), the file was left in a corrupted partial state, and the next run would either fail to read it or silently load malformed data.

This PR adds an atomic_write context manager in src/util/atomic_io.py that writes to a temporary file in the destination directory and atomically renames it into place with os.replace. On failure the temp file is removed, so an existing file is never clobbered by a partial write. Readers therefore only ever observe either the old complete file or the new complete file.

The issue referenced a df.to_csv() call that has since become df.to_parquet(), but the underlying non-atomic write remained and is fixed here.

Changes in this pull request

  • Add atomic_write context manager in src/util/atomic_io.py, exported from src.util
  • Use atomic writes for the parquet price cache in StockDataDownloader._download_single
  • Use atomic writes for stock_info.json in StockDataDownloader.save_stock_info
  • Use atomic writes for pickled models in Model.save_model and StackedStockPredictor.save_model
  • Add unit tests for atomic_write covering success, replacement, failure rollback and temp-file cleanup
  • Add a regression test that a failed save_stock_info preserves the existing cache file

🤖 Generated with Claude Code

Writes to the parquet price cache, stock_info.json and pickled model
files went directly to the final path, so a crash mid-write (OOM,
SIGKILL, disk full) left a corrupted file that the next run would read.

Add an atomic_write context manager in src/util that writes to a temp
file in the destination directory and renames it into place with
os.replace, removing the temp file on failure so existing files are
never clobbered by a partial write.

Fixes #127

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@github-actions github-actions Bot added the fix label Jun 10, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bug: Non-atomic file writes can corrupt cached data

1 participant