fix: never truncate a copy another run renames

On Windows a verified partial copy is closed before its rename. A second
run that claimed it in that moment truncated it and the first rename failed
with a sharing violation, reported as an unwritable folder. A complete
partial is now left whole there, the rename waits for a sharing violation
to clear, and a sharing violation no longer reads as a refused folder.
This commit is contained in:
Abdessamad Derraz committed 2026-10-21 05:16:12 +02:00
1 parent c638c12ca8
commit 9b1d378d2f
4 files changed
+183 -5

No files matched your search

+1 -1
View File
@@ -12,7 +12,7 @@
$ErrorActionPreference = "Stop"
$defaultInstallUrl = "https://raw.githubusercontent.com/Abdess/retrobios/main/install.py"
$defaultInstallSha256 = "973d64627af05b239555ab5a83a8902747b7a742bc1176f1b994cf7ac74774f9"
$defaultInstallSha256 = "007fffece97716aa9e1ed0606a8453f2da8a1262711aa68ca910673956b2b341"
$maximumInstallerBytes = 2MB
$installerExit = 0
$installer = if ($PSScriptRoot) { Join-Path $PSScriptRoot "install.py" } else { $null }
+87 -3
View File
@@ -2576,6 +2576,7 @@ def _download_one(
resumable = expected_size >= RESUME_FROM_SIZE
reason = "download failed"
refused = False
full_partial = False
for attempt in range(1, MAX_RETRIES + 1):
tmp_path: Path | None = None
counted = 0
@@ -2594,7 +2595,17 @@ def _download_one(
tmp_path = None
else:
offset = os.fstat(partial.fileno()).st_size
if offset >= expected_size:
if offset >= expected_size and _NO_RENAME_WHILE_OPEN:
# The run that verified this copy releases its lock
# to rename it: truncating it there destroyed that
# copy and failed the rename. It is left alone and
# this run writes a private file.
partial.close()
partial = None
tmp_path = None
offset = 0
full_partial = True
elif offset >= expected_size:
partial.truncate(0)
offset = 0
request = urllib.request.Request(url)
@@ -2665,11 +2676,15 @@ def _download_one(
# written with sudo has to stay readable by the emulator's user.
os.chmod(tmp_path, (0o777 if f.get("executable") else 0o666) & ~_UMASK)
_clear_read_only(dest)
if partial is not None and os.name == "nt":
if partial is not None and _NO_RENAME_WHILE_OPEN:
# Windows renames no open file. Elsewhere the lock is held
# until the copy has its final name.
partial.close()
os.replace(tmp_path, dest)
_replace_released(tmp_path, dest, expected)
else:
os.replace(tmp_path, dest)
if full_partial:
_drop_full_partial(dest, expected_size)
return f["dest"], True, ""
except _Interrupted:
@@ -2739,6 +2754,71 @@ def _download_one(
return f["dest"], False, reason
# Windows renames no file another handle holds open, and Python opens files
# without FILE_SHARE_DELETE: a locked partial copy is closed before its rename.
_NO_RENAME_WHILE_OPEN = os.name == "nt"
# Seconds a closed partial copy waits for a sharing violation to clear: a
# second run closing it again, or a scanner reading the new file.
RELEASE_WAIT = 30.0
_SHARING_VIOLATIONS = (32, 33)
def _replace_released(source: Path, dest: Path, expected: dict[str, str]) -> None:
"""Rename a verified copy whose lock was just released.
Another run may open the copy in that moment, which refuses the rename
with a sharing violation until it lets go. When the copy is gone, that
run placed the same file and removed it: the destination is checked
against the manifest's hashes before it counts as installed.
"""
deadline = time.monotonic() + RELEASE_WAIT
while True:
try:
os.replace(source, dest)
return
except FileNotFoundError:
if expected and dest.is_file() and _digest_file(dest, tuple(expected)) == expected:
return
raise
except PermissionError as exc:
if (getattr(exc, "winerror", None) not in _SHARING_VIOLATIONS
or time.monotonic() >= deadline):
raise
time.sleep(0.2)
def _drop_full_partial(dest: Path, expected_size: int) -> None:
"""Remove a complete partial copy this run left alone.
A copy at full size that nothing holds is what a run stopped between its
check and its rename leaves behind. The destination now holds the
verified file, so the copy is only clutter. One another run holds is
left to it.
"""
path = dest.parent / f".{dest.name}.part"
if not path.is_file():
return
try:
handle = _claim_partial(path)
except OSError:
return
if handle is None:
return
with handle:
complete = os.fstat(handle.fileno()).st_size >= expected_size
if complete:
try:
path.unlink()
except OSError:
pass # another run opened it meanwhile and finishes with it
else:
try:
if path.stat().st_size == 0:
path.unlink()
except OSError:
pass # created by the claim and taken since by another run
def _partial_size(path: Path) -> int:
"""Bytes already held by a partial copy. Anything but a plain file is dropped."""
try:
@@ -2823,6 +2903,10 @@ NOT_WRITABLE = "folder not writable"
def _write_refused(exc: OSError) -> bool:
# A sharing violation is a file another process holds open, which a
# later attempt can find released: no refusal of the folder.
if getattr(exc, "winerror", None) in _SHARING_VIOLATIONS:
return False
return isinstance(exc, PermissionError) or exc.errno == errno.EROFS
+1 -1
View File
@@ -18,7 +18,7 @@ esac
TEMP_INSTALLER=""
TEMP_DIRECTORY=""
DEFAULT_INSTALL_URL="https://raw.githubusercontent.com/Abdess/retrobios/main/install.py"
DEFAULT_INSTALL_SHA256="973d64627af05b239555ab5a83a8902747b7a742bc1176f1b994cf7ac74774f9"
DEFAULT_INSTALL_SHA256="007fffece97716aa9e1ed0606a8453f2da8a1262711aa68ca910673956b2b341"
MAX_INSTALLER_BYTES=2097152
cleanup() {
+94
View File
@@ -3586,5 +3586,99 @@ class DestAloneFollowsWhatIsFoundHere(unittest.TestCase):
self.assertIn("Which platform's files go to /media/card?", out)
class _FakeResponse:
"""What urlopen returns, serving one payload from its start."""
def __init__(self, payload: bytes) -> None:
self.status = 200
self.headers = {"Content-Length": str(len(payload))}
self._body = io.BytesIO(payload)
def __enter__(self) -> _FakeResponse:
return self
def __exit__(self, *_exc: object) -> None:
return None
def read(self, size: int = -1) -> bytes:
return self._body.read(size)
def _sharing_violation() -> PermissionError:
exc = PermissionError(13, "The process cannot access the file")
exc.winerror = 32
return exc
class ACompletePartialIsHandedOffOnWindows(unittest.TestCase):
"""Windows renames no open file, so the run that verified a large copy
released its lock before the rename. A second run claimed the copy in
that moment, found it complete and truncated it, and the first run's
rename failed with a sharing violation reported as "folder not
writable"."""
PAYLOAD = bytes(range(256)) * 200
ENTRY = {
"dest": "big.bin", "repo_path": "bios/big.bin", "size": len(PAYLOAD),
"sha256": hashlib.sha256(PAYLOAD).hexdigest(),
}
def _run(self, tmp: Path, seen: list[int]):
part = tmp / ".big.bin.part"
def urlopen(request, timeout=None):
seen.append(part.stat().st_size if part.exists() else -1)
return _FakeResponse(self.PAYLOAD)
with unittest.mock.patch.object(install, "RESUME_FROM_SIZE", 1024), \
unittest.mock.patch.object(install, "_NO_RENAME_WHILE_OPEN", True), \
unittest.mock.patch.object(install.urllib.request, "urlopen", urlopen):
return install._download_one(dict(self.ENTRY), tmp)
def test_the_other_runs_copy_is_left_whole(self):
tmp = Path(_tempdir(self))
(tmp / ".big.bin.part").write_bytes(self.PAYLOAD)
seen: list[int] = []
self.assertEqual(self._run(tmp, seen), ("big.bin", True, ""))
# The download ran while the complete copy was still whole.
self.assertEqual(seen, [len(self.PAYLOAD)])
self.assertEqual((tmp / "big.bin").read_bytes(), self.PAYLOAD)
# Nothing held it afterwards: it was a leftover, now removed.
self.assertFalse((tmp / ".big.bin.part").exists())
def test_the_rename_waits_for_the_other_run_to_let_go(self):
tmp = Path(_tempdir(self))
source = tmp / "copy"
source.write_bytes(b"x")
real_replace = os.replace
calls = []
def replace(src, dst):
calls.append(src)
if len(calls) < 3:
raise _sharing_violation()
return real_replace(src, dst)
with unittest.mock.patch.object(install.os, "replace", replace), \
unittest.mock.patch.object(install.time, "sleep"):
install._replace_released(source, tmp / "dest", {})
self.assertEqual(len(calls), 3)
self.assertEqual((tmp / "dest").read_bytes(), b"x")
def test_a_copy_the_other_run_placed_counts_once_verified(self):
tmp = Path(_tempdir(self))
dest = tmp / "dest"
dest.write_bytes(self.PAYLOAD)
expected = {"sha256": self.ENTRY["sha256"]}
install._replace_released(tmp / "gone", dest, expected)
dest.write_bytes(b"other bytes")
with self.assertRaises(FileNotFoundError):
install._replace_released(tmp / "gone", dest, expected)
def test_a_sharing_violation_is_no_refused_folder(self):
self.assertFalse(install._write_refused(_sharing_violation()))
self.assertTrue(install._write_refused(PermissionError(13, "denied")))
if __name__ == "__main__":
unittest.main()