Skip to content

Commit 3db6c19

Browse files
committed
Claim export destinations without clobbering races
1 parent 5c4bf8b commit 3db6c19

1 file changed

Lines changed: 41 additions & 12 deletions

File tree

src/openstatspec/spss/sav.py

Lines changed: 41 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -330,6 +330,32 @@ def _reserve_export_backup(destination: Path) -> Path:
330330
return Path(name)
331331

332332

333+
def _publish_staged_destination(
334+
*, staged: Path, destination: Path, backup: Path,
335+
state: dict[str, Any],
336+
) -> None:
337+
"""Publish without overwriting an entry created during export preparation."""
338+
state.update({
339+
"had_previous": False,
340+
"backup_installed": False,
341+
"published_identity": None,
342+
})
343+
if _path_entry_exists(destination):
344+
os.replace(destination, backup)
345+
state.update({"had_previous": True, "backup_installed": True})
346+
else:
347+
backup.unlink(missing_ok=True)
348+
349+
# The hard-link claim is atomic and fails if another process publishes the
350+
# destination after the check above. Staging is on the same filesystem.
351+
os.link(staged, destination)
352+
published_identity = _destination_identity(destination)
353+
if published_identity is None:
354+
raise FileNotFoundError("The published export destination disappeared.")
355+
state["published_identity"] = published_identity
356+
staged.unlink()
357+
358+
333359
def _restore_export_destination(
334360
*, destination: Path, backup: Path, had_previous: bool,
335361
expected_identity: tuple[int, int] | None | object = (
@@ -651,29 +677,32 @@ def export_sav_dataset(
651677
raise
652678
backup_installed = False
653679
try:
654-
if had_previous:
655-
os.replace(destination_path, backup)
656-
backup_installed = True
657-
os.replace(staged_destination, destination_path)
658-
published_identity = _destination_identity(destination_path)
659-
if published_identity is None:
660-
raise FileNotFoundError(
661-
"The published export destination disappeared."
662-
)
680+
_publish_staged_destination(
681+
staged=staged_destination,
682+
destination=destination_path,
683+
backup=backup,
684+
state=publication_state,
685+
)
686+
had_previous = publication_state["had_previous"]
687+
backup_installed = publication_state["backup_installed"]
663688
publication_state.update({
664689
"published": True,
665-
"published_identity": published_identity,
666690
"backup": backup,
667691
"staged": staged_destination,
668-
"had_previous": had_previous,
669692
"operation_id": operation_id,
670693
})
671694
except Exception as publish_error:
695+
had_previous = publication_state.get("had_previous", had_previous)
696+
backup_installed = publication_state.get(
697+
"backup_installed", backup_installed,
698+
)
699+
published_identity = publication_state.get("published_identity")
672700
if backup_installed or not had_previous:
673701
try:
674702
_restore_export_destination(
675703
destination=destination_path, backup=backup,
676-
had_previous=had_previous, expected_identity=None,
704+
had_previous=had_previous,
705+
expected_identity=published_identity,
677706
)
678707
except Exception as cleanup_error:
679708
_raise_export_cleanup_failed(

0 commit comments

Comments
 (0)