Skip to content

Commit 0703f84

Browse files
Write TSV, JSON and text files atomically
save_json(), write_python(), write_text(), write_tsv() and _write_tsv_simple() all opened the destination in 'w' mode, which truncates it before a single byte of the new content is written. A crash, a full disk or a kill in that window leaves the file empty or half written and the previous contents are gone. cluster_group.tsv is the one that hurts, since it holds manual curation labels that cannot be recomputed. Add _atomic_open(), a context manager that writes to a temporary file in the destination directory and moves it into place with os.replace() on a clean exit, and route all five writers through it. The temporary file is given the permissions of the file it replaces, or the permissions open() would have used for a new file, since tempfile creates its files with 0600.
1 parent d99464b commit 0703f84

3 files changed

Lines changed: 149 additions & 13 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
1414
- Development checkouts now report a development version with a Git commit suffix.
1515
- Template datasets without waveform templates can be reloaded after cluster assignments change,
1616
and stored spike-waveform subsets provide their waveform sample count.
17+
- TSV, JSON, text and `params.py` files are written atomically, so a crash during a save no longer
18+
truncates the file it was replacing, for instance `cluster_group.tsv`.
1719

1820
## [2.7.0] 2025-12-10
1921

‎phylib/utils/_misc.py‎

Lines changed: 56 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -8,13 +8,15 @@
88
#------------------------------------------------------------------------------
99

1010
import base64
11+
from contextlib import contextmanager
1112
import csv
1213
from importlib import import_module
1314
import json
1415
import logging
1516
import os
1617
from pathlib import Path
1718
import subprocess
19+
import tempfile
1820
from textwrap import dedent
1921

2022
import numpy as np
@@ -24,6 +26,54 @@
2426
logger = logging.getLogger(__name__)
2527

2628

29+
#------------------------------------------------------------------------------
30+
# Atomic file writing
31+
#------------------------------------------------------------------------------
32+
33+
def _copy_file_mode(src, dst):
34+
"""Give `src` the permissions `dst` has, or the permissions a new file would get."""
35+
try:
36+
mode = os.stat(dst).st_mode & 0o777
37+
except OSError:
38+
# The destination does not exist yet: reproduce what open() would have done, since
39+
# tempfile creates its files with 0600 and saving must not make the file private.
40+
umask = os.umask(0)
41+
os.umask(umask)
42+
mode = 0o666 & ~umask
43+
os.chmod(src, mode)
44+
45+
46+
@contextmanager
47+
def _atomic_open(path, mode='w', **kwargs):
48+
"""Open a file for writing without ever truncating the file it replaces.
49+
50+
The contents are written to a temporary file in the same directory as the destination, and
51+
that file is moved into place with `os.replace()` once the `with` block completes, which is
52+
atomic as long as both paths are on the same filesystem. A reader therefore sees either the
53+
old file or the new one, never a half-written one. If the block raises, the temporary file
54+
is removed and any existing file is left untouched.
55+
56+
"""
57+
path = Path(path)
58+
ensure_dir_exists(path.parent)
59+
# NOTE: the temporary file is hidden and does not end in a data file extension, so that a
60+
# file left behind by a hard kill is not picked up by the globs used to discover datasets.
61+
fd, tmp = tempfile.mkstemp(dir=str(path.parent), prefix='.' + path.name + '.', suffix='.tmp')
62+
try:
63+
with os.fdopen(fd, mode, **kwargs) as f:
64+
yield f
65+
f.flush()
66+
os.fsync(f.fileno())
67+
_copy_file_mode(tmp, path)
68+
os.replace(tmp, path)
69+
except BaseException:
70+
try:
71+
os.remove(tmp)
72+
except OSError: # pragma: no cover
73+
pass
74+
raise
75+
76+
2777
#------------------------------------------------------------------------------
2878
# JSON utility functions
2979
#------------------------------------------------------------------------------
@@ -128,9 +178,7 @@ def save_json(path, data):
128178
"""
129179
assert isinstance(data, dict)
130180
data = _stringify_keys(data)
131-
path = Path(path)
132-
ensure_dir_exists(path.parent)
133-
with path.open('w') as f:
181+
with _atomic_open(path) as f:
134182
json.dump(data, f, cls=_CustomEncoder, indent=2, sort_keys=True)
135183

136184

@@ -190,7 +238,7 @@ def write_python(path, data):
190238
-------
191239
192240
"""
193-
with open(path, 'w') as f:
241+
with _atomic_open(path) as f:
194242
for k, v in data.items():
195243
if isinstance(v, str):
196244
v = '"%s"' % v
@@ -206,9 +254,8 @@ def read_text(path):
206254
def write_text(path, contents):
207255
"""Write a text file."""
208256
contents = dedent(contents)
209-
path = Path(path)
210-
ensure_dir_exists(path.parent)
211-
path.write_text(contents)
257+
with _atomic_open(path) as f:
258+
f.write(contents)
212259

213260

214261
def _try_make_number(value):
@@ -266,9 +313,8 @@ def write_tsv(path, data, first_field=None, exclude_fields=(), n_significant_fig
266313
267314
"""
268315
path = Path(path)
269-
ensure_dir_exists(path.parent)
270316
delimiter = '\t' if path.suffix == '.tsv' else ','
271-
with path.open('w', newline='') as f:
317+
with _atomic_open(path, newline='') as f:
272318
if not data:
273319
logger.info("Data was empty when writing %s.", path)
274320
return
@@ -327,9 +373,8 @@ def _write_tsv_simple(path, field_name, data):
327373
328374
"""
329375
path = Path(path)
330-
ensure_dir_exists(path.parent)
331376
delimiter = '\t' if path.suffix == '.tsv' else ','
332-
with path.open('w', newline='') as f:
377+
with _atomic_open(path, newline='') as f:
333378
writer = csv.writer(f, delimiter=delimiter)
334379
writer.writerow(['cluster_id', field_name])
335380
writer.writerows([(cluster_id, data[cluster_id]) for cluster_id in sorted(data)])

‎phylib/utils/tests/test_misc.py‎

Lines changed: 91 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,15 +6,19 @@
66
# Imports
77
#------------------------------------------------------------------------------
88

9+
import csv
10+
import json
11+
import os
912
import re
13+
import stat
1014

1115
import numpy as np
1216
from numpy.testing import assert_array_equal as ae
1317
from pytest import raises, mark
1418

1519
from .._misc import (
16-
_git_version, load_json, save_json, load_pickle, save_pickle, read_python, write_python,
17-
read_text, write_text, _read_tsv_simple, _write_tsv_simple, read_tsv, write_tsv,
20+
_atomic_open, _git_version, load_json, save_json, load_pickle, save_pickle, read_python,
21+
write_python, read_text, write_text, _read_tsv_simple, _write_tsv_simple, read_tsv, write_tsv,
1822
_pretty_floats, _encode_qbytearray, _decode_qbytearray, _fullname, _load_from_fullname)
1923

2024

@@ -137,6 +141,91 @@ def test_write_tsv(tempdir):
137141
assert read_tsv(path) == data
138142

139143

144+
class _Boom(Exception):
145+
pass
146+
147+
148+
def test_atomic_open(tempdir):
149+
path = tempdir / 'test_dir/test.txt'
150+
151+
with _atomic_open(path) as f:
152+
f.write('hello')
153+
assert read_text(path) == 'hello'
154+
# The temporary file must not be left behind.
155+
assert sorted(p.name for p in path.parent.iterdir()) == ['test.txt']
156+
157+
# A new file gets the permissions open() would have given it, not the 0600 of a
158+
# temporary file.
159+
umask = os.umask(0)
160+
os.umask(umask)
161+
assert stat.S_IMODE(path.stat().st_mode) == 0o666 & ~umask
162+
163+
# The permissions of an existing file are preserved.
164+
path.chmod(0o640)
165+
with _atomic_open(path) as f:
166+
f.write('world')
167+
assert read_text(path) == 'world'
168+
assert stat.S_IMODE(path.stat().st_mode) == 0o640
169+
170+
# A failure half-way through leaves the previous file untouched and no leftovers.
171+
with raises(_Boom):
172+
with _atomic_open(path) as f:
173+
f.write('this should never be visible')
174+
raise _Boom()
175+
assert read_text(path) == 'world'
176+
assert sorted(p.name for p in path.parent.iterdir()) == ['test.txt']
177+
178+
179+
@mark.parametrize('writer', ['write_tsv', '_write_tsv_simple'])
180+
def test_write_tsv_atomic(tempdir, monkeypatch, writer):
181+
# Curation data such as cluster_group.tsv must survive a crash during a save.
182+
path = tempdir / 'cluster_group.tsv'
183+
if writer == 'write_tsv':
184+
def _write(value):
185+
write_tsv(path, [{'cluster_id': 2, 'group': value}], first_field='cluster_id')
186+
else:
187+
def _write(value):
188+
_write_tsv_simple(path, 'group', {2: value})
189+
190+
_write('good')
191+
before = path.read_bytes()
192+
assert before
193+
files_before = sorted(p.name for p in tempdir.iterdir())
194+
195+
real_writer = csv.writer
196+
197+
def _failing_writer(f, **kwargs):
198+
# Mimic a crash after part of the file has been written.
199+
real_writer(f, **kwargs).writerow(['cluster_id', 'group'])
200+
raise _Boom()
201+
202+
monkeypatch.setattr(csv, 'writer', _failing_writer)
203+
with raises(_Boom):
204+
_write('mua')
205+
206+
assert path.read_bytes() == before
207+
assert sorted(p.name for p in tempdir.iterdir()) == files_before
208+
209+
210+
def test_save_json_atomic(tempdir, monkeypatch):
211+
path = tempdir / 'test.json'
212+
save_json(path, {'a': 1})
213+
before = path.read_bytes()
214+
assert before
215+
files_before = sorted(p.name for p in tempdir.iterdir())
216+
217+
def _failing_dump(data, f, **kwargs):
218+
f.write('{"a":')
219+
raise _Boom()
220+
221+
monkeypatch.setattr(json, 'dump', _failing_dump)
222+
with raises(_Boom):
223+
save_json(path, {'a': 2})
224+
225+
assert path.read_bytes() == before
226+
assert sorted(p.name for p in tempdir.iterdir()) == files_before
227+
228+
140229
def test_git_version():
141230
v = _git_version()
142231
assert re.match(r'^\+git\.[0-9a-f]{8}(?:\.dirty)?$', v)

0 commit comments

Comments
 (0)