Skip to content

Don't truncate existing file when save fails - #521

Merged
JarrettSJohnson merged 1 commit into
masterfrom
fix-520-atomic-save
Oct 7, 2026
Merged

JarrettSJohnson merged 1 commit into
masterfrom
fix-520-atomic-save

Conversation

@JarrettSJohnson

Copy link
Copy Markdown
Member

Fixes #520

Problem

save opened the target with 'wb', which truncates an existing file before the new contents are written. Any failure after the open (disk full, file size limit, process killed) left the previous file empty. For .pse/.pze/.pse.gz sessions, that means losing scenes, views, selections and coloring. I reproduced this on master for all three session formats with ulimit -f 0.

Fix

A new helper, _write_file_atomic, writes to a temporary file in the same directory and then os.replace()s it over the target:

  • An existing file's permission bits are copied to the replacement (shutil.copymode). The temp file is created with open(..., 'xb'), so new files still respect the umask.
  • If anything fails, the temp file is removed and the error is re-raised.
  • Symlinks, non-regular files (e.g. /dev/stdout), read-only files and writable files in non-writable directories fall back to the previous direct write. Read-only files are still refused instead of being silently replaced.
  • gzip output is now compressed in memory with gzip.compress, as bz2 already was. Side effect: the gzip header no longer stores the original filename, which only matters to gunzip -N.

Not covered: exporters that write the file themselves (e.g. png, mmod, pkl). There is no fsync, so power loss is out of scope, as in the issue.

Testing

  • New tests in testing/tests/api/test_exporting.py:
    • A simulated ENOSPC partway through the write leaves .pse/.pse.gz/.pdb unchanged, with no temp files left behind. These fail on master.
    • File permissions are kept, read-only files are still refused, and saving to a symlink writes to the file it points to.
  • With the issue's ulimit -f 0 steps, .pse, .pse.gz and .pze stay byte-for-byte the same. Compressed sessions still load.
  • tests/api/exporting.py and tests/api/importing.py via testing.py: 141 tests, all pass except testglTF, which errors locally because collada2gltf is not installed.

🤖 Generated with Claude Code

`save` opened the target with 'wb', truncating it before the new
contents were written. A failed write (disk full, file size limit,
process killed) left an existing session or structure file empty.

Write to a temporary file in the same directory and os.replace() it
over the target, keeping the file mode. Symlinks, non-regular files,
read-only files and non-writable directories fall back to the
previous direct write. gzip output is now compressed in memory like
bz2, which drops the original filename from the gzip header.

Fixes #520

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@JarrettSJohnson
JarrettSJohnson merged commit edcda80 into master Oct 7, 2026
23 of 24 checks passed
JarrettSJohnson added a commit that referenced this pull request Oct 7, 2026
get_gltf writes the file itself, so it bypassed the atomic write that
save gained in #521 and a failed write could still truncate an existing
.gltf file. Use exporting._write_file_atomic, and cover glb/gltf in the
save failure test (requires_capability now returns a pytest mark, so it
also works in pytest.param).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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.

save of a session truncates the existing .pse before writing it, so a failed write leaves it empty

1 participant