Skip to content

Commit 745794d

Browse files
authored
Merge pull request #2069 from DeusData/fix/test-wave-atomic-barrier-writes
test: write scheduler barrier files atomically
2 parents 4efedf2 + f895152 commit 745794d

2 files changed

Lines changed: 46 additions & 3 deletions

File tree

‎scripts/run-test-wave.py‎

Lines changed: 29 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
import signal
1818
import subprocess
1919
import sys
20+
import tempfile
2021
import time
2122
from dataclasses import dataclass
2223

@@ -105,6 +106,31 @@ def append_log(path: pathlib.Path, message: str) -> None:
105106
stream.write("\n")
106107

107108

109+
def publish_barrier_file(path: pathlib.Path, text: str) -> None:
110+
"""Publish a barrier file so a poller sees either no file or its content.
111+
112+
Path.write_text creates and truncates before it writes, so a reader that
113+
polls for existence and then parses the content can observe the zero-byte
114+
window in between. Write next to the destination and rename into place.
115+
"""
116+
with tempfile.NamedTemporaryFile(
117+
mode="w",
118+
encoding="utf-8",
119+
newline="\n",
120+
dir=path.parent,
121+
prefix=f".{path.name}.",
122+
delete=False,
123+
) as temporary:
124+
temporary.write(text)
125+
temporary.flush()
126+
temporary_path = pathlib.Path(temporary.name)
127+
try:
128+
os.replace(temporary_path, path)
129+
except BaseException:
130+
temporary_path.unlink(missing_ok=True)
131+
raise
132+
133+
108134
def start_suite(
109135
suite: str,
110136
runner_command: list[str],
@@ -288,12 +314,12 @@ def wait_for_test_pre_terminate_barrier(
288314
ready = barrier_dir / f"{active.name}.ready"
289315
leader_exited = barrier_dir / f"{active.name}.leader-exited"
290316
release = barrier_dir / f"{active.name}.release"
291-
ready.write_text(f"{active.process.pid}\n", encoding="utf-8")
317+
publish_barrier_file(ready, f"{active.process.pid}\n")
292318
deadline = time.monotonic() + 10
293319
while not release.exists():
294320
returncode = active.process.poll()
295321
if returncode is not None and not leader_exited.exists():
296-
leader_exited.write_text(f"{returncode}\n", encoding="utf-8")
322+
publish_barrier_file(leader_exited, f"{returncode}\n")
297323
if time.monotonic() >= deadline:
298324
raise RuntimeError(
299325
f"test pre-terminate barrier for {active.name!r} was not released"
@@ -312,7 +338,7 @@ def wait_for_test_post_exit_barrier(
312338
return
313339
ready = barrier_dir / f"{suite}.ready"
314340
release = barrier_dir / f"{suite}.release"
315-
ready.write_text("child exited; result intentionally not recorded\n", encoding="utf-8")
341+
publish_barrier_file(ready, "child exited; result intentionally not recorded\n")
316342
deadline = time.monotonic() + 10
317343
while not release.exists():
318344
if time.monotonic() >= deadline:

‎tests/test_parallel_harness_contract.sh‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,23 @@ if [[ "$probe_sites" == *kill_grace* ]]; then
4545
exit 1
4646
fi
4747

48+
# Barrier files must be published atomically. Path.write_text creates and
49+
# truncates before it writes, so a poller that saw `<suite>.ready` appear and
50+
# then parsed the leader pid could read the zero-byte window and fail on
51+
# int("") -- a scheduler-side race surfacing as a harness flake. Asserted
52+
# structurally: no barrier file is written in place, and the scheduler renames
53+
# a same-directory temp file onto the destination instead.
54+
barrier_writes=$(grep -nE '^[[:space:]]*(ready|leader_exited)\.write_text\(' "$scheduler" || true)
55+
if [ -n "$barrier_writes" ]; then
56+
echo "FAIL: scheduler barrier files are written in place (non-atomic):" >&2
57+
echo "$barrier_writes" >&2
58+
exit 1
59+
fi
60+
if ! grep -Fq 'os.replace(' "$scheduler"; then
61+
echo "FAIL: scheduler does not rename barrier files into place" >&2
62+
exit 1
63+
fi
64+
4865
python3 - "$scheduler" <<'PROBE'
4966
from __future__ import annotations
5067

0 commit comments

Comments
 (0)