Two MeshTerms can share a machine, but not a directory
Every copy on a machine reads and writes the same ~/.meshterm — a checkout, a downloaded build, a second one for a second radio — and nothing about how it was installed changes that. Two at once will not corrupt anything, but each holds the JSON stores in memory and rewrites them whole, so the second to save discards the first's changes. A lost contact is much harder to notice than a crash. The interactive session now takes an exclusive lock on the directory it is using, and a second one is refused with the fix in the message: set MESHTERM_HOME. The lock belongs to the operating system rather than to a process id written in a file, because a pid file has to answer "is that still alive?", the answer differs per platform, and it is wrong once a reboot recycles the number — so a crash leaves a lock nobody can explain and everyone learns to delete it. One-shot subcommands do not take it; blocking `meshterm contacts` because a menu is open in another window would be an obstacle rather than a guard. The scratch-file race is fixed at the same time, and it was ten places rather than the three I first counted: every store under core/ carried the same three lines, writing to a fixed `.tmp` name beside its file. Two processes saving together took turns inside that one scratch file and then each renamed whatever was in it over the real one — the rename is atomic, the thing being renamed was not. They now go through one `write_atomically`, whose temporary carries the writing process's id, and which cleans up after a failed write instead of leaving a partial file behind. And the Windows console error is answered rather than raised. prompt_toolkit needs a Win32 console; Git Bash, MSYS and Cygwin are not one, and its complaint arrives as a traceback that reads like a broken application when the answer is "open a different terminal". It now says that, and points out that every screen has a subcommand which pipes perfectly well. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
1 parent
9b2c5527f8
commit
e2161fd191
15 files changed
+339
-47
No files matched your search
@@ -17,6 +17,25 @@ allowed to change behaviour, not just add to it.
|
||||
|
||||
## [Unreleased]
|
||||
|
||||
### Added
|
||||
|
||||
- One interactive MeshTerm per data directory. A second one is turned away with the way
|
||||
out in the message, rather than quietly overwriting the first one's contacts and
|
||||
settings. The lock is held by the operating system, so a crash releases it — a process
|
||||
id in a file would leave a lock nobody could explain.
|
||||
- A readable answer when there is no Windows console. Git Bash, MSYS and Cygwin are not
|
||||
Windows consoles, and prompt_toolkit's own error says so in the language of its
|
||||
internals, arriving as a traceback that reads like a broken app rather than the
|
||||
instruction it is.
|
||||
|
||||
### Fixed
|
||||
|
||||
- All ten stores write through one helper, and the file each writes to first is named
|
||||
for the process writing it. They shared a fixed `.tmp` name, so two copies of MeshTerm
|
||||
saving at the same moment took turns inside one scratch file and then each renamed
|
||||
whatever was in it over the real one. The rename was always atomic; what was being
|
||||
renamed was not.
|
||||
|
||||
## [0.2.2] — 2026-09-07
|
||||
|
||||
### Fixed
|
||||
|
||||
+41
-1
@@ -13,6 +13,7 @@ import logging
|
||||
from pathlib import Path
|
||||
|
||||
import typer
|
||||
from rich.console import Console
|
||||
|
||||
from .context import AppContext
|
||||
from .core.admin_store import AdminStore
|
||||
@@ -20,6 +21,7 @@ from .core.config import Settings
|
||||
from .core.connection import DeviceCommandError, is_connection_lost
|
||||
from .core.device_config import DeviceConfigError
|
||||
from .core.device_store import DeviceStore
|
||||
from .core.instancelock import InstanceBusy, hold_instance_lock
|
||||
from .core.preferences import PreferenceError
|
||||
from .core.selection import DeviceSelectionError
|
||||
from .persistence.logging import configure_logging
|
||||
@@ -166,7 +168,45 @@ def main_callback(
|
||||
if ctx.invoked_subcommand is None:
|
||||
from .ui.menu import run_menu
|
||||
|
||||
asyncio.run(_drive(run_menu(app_ctx), app_ctx))
|
||||
# The interactive session is the one that holds the stores in memory and rewrites
|
||||
# them whole, so it is the one that claims the directory. One-shot subcommands are
|
||||
# brief and mostly read; blocking `meshterm contacts` because a menu is open in
|
||||
# another window would be an obstacle rather than a guard.
|
||||
try:
|
||||
with hold_instance_lock(settings.config_dir):
|
||||
asyncio.run(_drive(run_menu(app_ctx), app_ctx))
|
||||
except InstanceBusy as exc:
|
||||
console.print(f"[err]✗[/err] {exc}")
|
||||
raise typer.Exit(code=1) from None
|
||||
except Exception as exc:
|
||||
if type(exc).__name__ != "NoConsoleScreenBufferError":
|
||||
raise
|
||||
# prompt_toolkit needs a real Win32 console and reports its absence in the
|
||||
# language of its own internals. A traceback reads as "this app is broken"
|
||||
# when the answer is "open a different terminal", so answer that instead.
|
||||
_report_no_windows_console(console)
|
||||
raise typer.Exit(code=1) from None
|
||||
|
||||
|
||||
def _report_no_windows_console(console: Console) -> None:
|
||||
"""Explain a console MeshTerm cannot draw in, and how to get one it can.
|
||||
|
||||
Reached when prompt_toolkit finds no Win32 console screen buffer: a Git Bash, MSYS or
|
||||
Cygwin shell — which announce themselves as ``xterm`` and are not Windows consoles at
|
||||
all — or a process whose output has been redirected away from one.
|
||||
"""
|
||||
console.print("[err]✗[/err] MeshTerm needs a Windows console, and could not find one.")
|
||||
console.print()
|
||||
console.print("That usually means one of two things:")
|
||||
console.print()
|
||||
console.print(" • You are in Git Bash, MSYS or Cygwin. Those are not Windows consoles.")
|
||||
console.print(" Open [accent]Windows Terminal[/accent] or [accent]PowerShell[/accent]")
|
||||
console.print(" and run it from there.")
|
||||
console.print(" • The output is piped or redirected somewhere. The full-screen menu")
|
||||
console.print(" needs the terminal itself.")
|
||||
console.print()
|
||||
console.print("Every screen also has a subcommand, and those pipe fine —")
|
||||
console.print("try [accent]meshterm --help[/accent].")
|
||||
|
||||
|
||||
@app.command(name="platform")
|
||||
|
||||
@@ -14,11 +14,10 @@ a multi-user secret store; treat the file accordingly.
|
||||
from __future__ import annotations
|
||||
|
||||
import json
|
||||
import os
|
||||
import stat
|
||||
from dataclasses import dataclass
|
||||
from pathlib import Path
|
||||
|
||||
from .atomicwrite import write_atomically
|
||||
from .models import Contact, LoginResult, utcnow
|
||||
|
||||
|
||||
@@ -139,11 +138,4 @@ class AdminStore:
|
||||
|
||||
def _write(self, records: dict[str, dict]) -> None:
|
||||
"""Persist ``records`` atomically with owner-only permissions where supported."""
|
||||
self._path.parent.mkdir(parents=True, exist_ok=True)
|
||||
tmp = self._path.with_suffix(self._path.suffix + ".tmp")
|
||||
tmp.write_text(json.dumps(records, indent=2), encoding="utf-8")
|
||||
try: # best-effort: not all platforms/filesystems honor chmod
|
||||
os.chmod(tmp, stat.S_IRUSR | stat.S_IWUSR)
|
||||
except OSError: # pragma: no cover - platform-dependent
|
||||
pass
|
||||
tmp.replace(self._path)
|
||||
write_atomically(self._path, json.dumps(records, indent=2), owner_only=True)
|
||||
@@ -20,6 +20,7 @@ from dataclasses import dataclass
|
||||
from datetime import datetime
|
||||
from pathlib import Path
|
||||
|
||||
from .atomicwrite import write_atomically
|
||||
from .models import utcnow
|
||||
|
||||
#: The direct (zero-hop) cadence choices offered in the editor, in hours.
|
||||
@@ -190,10 +191,7 @@ class AdvertStore:
|
||||
|
||||
def _write(self, records: dict[str, dict]) -> None:
|
||||
"""Persist ``records`` atomically (crash mid-write keeps the previous file)."""
|
||||
self._path.parent.mkdir(parents=True, exist_ok=True)
|
||||
tmp = self._path.with_suffix(self._path.suffix + ".tmp")
|
||||
tmp.write_text(json.dumps(records, indent=2), encoding="utf-8")
|
||||
tmp.replace(self._path)
|
||||
write_atomically(self._path, json.dumps(records, indent=2))
|
||||
|
||||
|
||||
def _as_hours(value: object, default: int) -> int:
|
||||
|
||||
@@ -0,0 +1,54 @@
|
||||
"""THE way MeshTerm replaces a file it owns: all of the new contents, or none of them.
|
||||
|
||||
Every store under ``core/`` keeps its state in one file that it rewrites whole. Writing
|
||||
in place means a crash — or a laptop lid closing — halfway through leaves a truncated
|
||||
file where a contact list used to be, so each write goes to a neighbouring temporary file
|
||||
first and lands with a single rename, which the filesystem either does or doesn't.
|
||||
|
||||
The neighbouring file's name carries the writing process's id. That is not fussiness:
|
||||
MeshTerm's data directory is shared by every copy on the machine — a checkout, a
|
||||
downloaded build, a second instance for a second radio — and a fixed ``.tmp`` name means
|
||||
two of them writing at the same moment take turns clobbering one scratch file, then each
|
||||
rename whatever is in it over the real one. The rename is atomic; the thing being renamed
|
||||
was not. Per-process names make the two writes independent again, and the last rename
|
||||
simply wins, which is a lost update rather than a corrupted file.
|
||||
|
||||
``MESHTERM_HOME`` is the way to avoid sharing the directory in the first place — see
|
||||
:func:`meshterm.core.config.default_config_dir`.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import os
|
||||
import stat
|
||||
from pathlib import Path
|
||||
|
||||
__all__ = ["write_atomically"]
|
||||
|
||||
|
||||
def write_atomically(path: Path, text: str, *, owner_only: bool = False) -> None:
|
||||
"""Replace ``path`` with ``text`` in one step, creating parent directories as needed.
|
||||
|
||||
Args:
|
||||
path: The file to replace. Its directory is created if it does not exist.
|
||||
text: The complete new contents, written as UTF-8.
|
||||
owner_only: Restrict the file to the owner (``0600``) before it is put in place,
|
||||
for anything holding a credential. Applied to the temporary file rather than
|
||||
the destination, so there is no moment where the real file exists with wider
|
||||
permissions. Best-effort: not every platform or filesystem honours it.
|
||||
"""
|
||||
path.parent.mkdir(parents=True, exist_ok=True)
|
||||
tmp = path.with_suffix(f"{path.suffix}.{os.getpid()}.tmp")
|
||||
try:
|
||||
tmp.write_text(text, encoding="utf-8")
|
||||
if owner_only:
|
||||
try:
|
||||
os.chmod(tmp, stat.S_IRUSR | stat.S_IWUSR)
|
||||
except OSError: # pragma: no cover - platform-dependent
|
||||
pass
|
||||
tmp.replace(path)
|
||||
except BaseException:
|
||||
# A failed write leaves no litter behind. `missing_ok` because the failure may
|
||||
# well have been the write that would have created it.
|
||||
tmp.unlink(missing_ok=True)
|
||||
raise
|
||||
@@ -30,6 +30,7 @@ from dataclasses import dataclass
|
||||
from pathlib import Path
|
||||
from typing import TYPE_CHECKING
|
||||
|
||||
from .atomicwrite import write_atomically
|
||||
from .channels import (
|
||||
CHANNEL_SECRET_BYTES,
|
||||
CHANNEL_SLOT_EMPTY_RUN,
|
||||
@@ -156,10 +157,7 @@ class ChannelStore:
|
||||
if channels
|
||||
}
|
||||
}
|
||||
self._path.parent.mkdir(parents=True, exist_ok=True)
|
||||
tmp = self._path.with_suffix(self._path.suffix + ".tmp")
|
||||
tmp.write_text(json.dumps(data, indent=2), encoding="utf-8")
|
||||
tmp.replace(self._path)
|
||||
write_atomically(self._path, json.dumps(data, indent=2))
|
||||
|
||||
|
||||
def _channel_from_json(entry: object) -> RememberedChannel | None:
|
||||
|
||||
@@ -49,6 +49,7 @@ import json
|
||||
from dataclasses import dataclass
|
||||
from pathlib import Path
|
||||
|
||||
from .atomicwrite import write_atomically
|
||||
from .models import Contact, advert_time
|
||||
|
||||
|
||||
@@ -332,10 +333,7 @@ class ContactStore:
|
||||
if contacts
|
||||
}
|
||||
}
|
||||
self._path.parent.mkdir(parents=True, exist_ok=True)
|
||||
tmp = self._path.with_suffix(self._path.suffix + ".tmp")
|
||||
tmp.write_text(json.dumps(data, indent=2), encoding="utf-8")
|
||||
tmp.replace(self._path)
|
||||
write_atomically(self._path, json.dumps(data, indent=2))
|
||||
|
||||
|
||||
def _contact_to_json(contact: RememberedContact) -> dict:
|
||||
|
||||
@@ -19,6 +19,7 @@ from dataclasses import dataclass
|
||||
from datetime import datetime
|
||||
from pathlib import Path
|
||||
|
||||
from .atomicwrite import write_atomically
|
||||
from .models import utcnow
|
||||
|
||||
#: Message states: waiting in the outbox, landed, or abandoned after the retry budget.
|
||||
@@ -253,10 +254,7 @@ class CourierStore:
|
||||
for m in self._load()
|
||||
],
|
||||
}
|
||||
self._path.parent.mkdir(parents=True, exist_ok=True)
|
||||
tmp = self._path.with_suffix(self._path.suffix + ".tmp")
|
||||
tmp.write_text(json.dumps(data, indent=2), encoding="utf-8")
|
||||
tmp.replace(self._path)
|
||||
write_atomically(self._path, json.dumps(data, indent=2))
|
||||
|
||||
|
||||
def _as_int(value: object, default: int) -> int:
|
||||
|
||||
@@ -19,6 +19,7 @@ import json
|
||||
from dataclasses import dataclass
|
||||
from pathlib import Path
|
||||
|
||||
from .atomicwrite import write_atomically
|
||||
from .discovery import (
|
||||
TRANSPORT_BLE,
|
||||
TRANSPORT_SERIAL,
|
||||
@@ -259,7 +260,4 @@ class DeviceStore:
|
||||
},
|
||||
"last": last,
|
||||
}
|
||||
# Atomic-ish replace so a crash mid-write can't truncate the existing registry.
|
||||
tmp = self._path.with_suffix(self._path.suffix + ".tmp")
|
||||
tmp.write_text(json.dumps(payload, indent=2), encoding="utf-8")
|
||||
tmp.replace(self._path)
|
||||
write_atomically(self._path, json.dumps(payload, indent=2))
|
||||
@@ -0,0 +1,94 @@
|
||||
"""One interactive MeshTerm per data directory, and a clear answer when you want two.
|
||||
|
||||
Every copy of MeshTerm on a machine reads and writes the same ``~/.meshterm`` — a
|
||||
checkout, a downloaded build, a second instance for a second radio. Nothing about how it
|
||||
was installed changes that. Two running at once will not corrupt anything (the database
|
||||
is in WAL mode and every file lands by rename) but each holds the JSON stores in memory
|
||||
and rewrites them whole, so the second one to save quietly discards the first one's
|
||||
changes. A lost contact is much harder to notice than a crash.
|
||||
|
||||
So the interactive session takes an exclusive lock on the directory it is using, and a
|
||||
second one is turned away with the fix in the message: set ``MESHTERM_HOME``.
|
||||
|
||||
The lock is held by the operating system rather than written down as a process id in a
|
||||
file. A pid file has to answer "is that process still alive?", the answer is different on
|
||||
every platform, and it is wrong after a reboot recycles the number — so a crash leaves a
|
||||
lock nobody can explain and everybody works around. An OS lock is released when the
|
||||
process ends, however it ends.
|
||||
|
||||
One-shot CLI subcommands do not take it. They are brief, mostly read, and blocking
|
||||
``meshterm contacts`` because a menu is open elsewhere would be an obstacle rather than a
|
||||
guard.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import sys
|
||||
from collections.abc import Iterator
|
||||
from contextlib import contextmanager
|
||||
from pathlib import Path
|
||||
|
||||
__all__ = ["InstanceBusy", "LOCK_NAME", "hold_instance_lock"]
|
||||
|
||||
#: The lock file, kept beside the data it guards. Empty; only the lock on it matters.
|
||||
LOCK_NAME = ".lock"
|
||||
|
||||
|
||||
class InstanceBusy(RuntimeError):
|
||||
"""Raised when another interactive MeshTerm already holds this data directory."""
|
||||
|
||||
def __init__(self, config_dir: Path) -> None:
|
||||
"""Build the message, which is mostly the way out of the situation."""
|
||||
super().__init__(
|
||||
f"Another MeshTerm is already using {config_dir}.\n\n"
|
||||
"Two of them sharing one directory will quietly overwrite each other's\n"
|
||||
"contacts and settings, so this one stopped instead.\n\n"
|
||||
"To run a second one — a downloaded build beside your own, or a second\n"
|
||||
"radio — give it a directory of its own:\n\n"
|
||||
" MESHTERM_HOME=~/meshterm-other meshterm\n"
|
||||
)
|
||||
self.config_dir = config_dir
|
||||
|
||||
|
||||
def _try_lock(handle) -> bool:
|
||||
"""Take an exclusive, non-blocking lock on an open file; ``False`` if someone has it."""
|
||||
if sys.platform == "win32":
|
||||
import msvcrt
|
||||
|
||||
try:
|
||||
msvcrt.locking(handle.fileno(), msvcrt.LK_NBLCK, 1)
|
||||
except OSError:
|
||||
return False
|
||||
return True
|
||||
|
||||
import fcntl
|
||||
|
||||
try:
|
||||
fcntl.flock(handle.fileno(), fcntl.LOCK_EX | fcntl.LOCK_NB)
|
||||
except OSError:
|
||||
return False
|
||||
return True
|
||||
|
||||
|
||||
@contextmanager
|
||||
def hold_instance_lock(config_dir: Path) -> Iterator[None]:
|
||||
"""Hold ``config_dir`` for this process, releasing it however the process ends.
|
||||
|
||||
Args:
|
||||
config_dir: The data directory to claim.
|
||||
|
||||
Raises:
|
||||
InstanceBusy: Another interactive MeshTerm holds it.
|
||||
"""
|
||||
config_dir.mkdir(parents=True, exist_ok=True)
|
||||
path = config_dir / LOCK_NAME
|
||||
handle = open(path, "a+b") # noqa: SIM115 - closed in the finally below
|
||||
try:
|
||||
if not _try_lock(handle):
|
||||
raise InstanceBusy(config_dir)
|
||||
yield
|
||||
finally:
|
||||
# Closing releases the lock on every platform. The file itself stays: deleting it
|
||||
# would race a second process that has already opened it and is about to lock,
|
||||
# which is how one directory ends up handed to two of them.
|
||||
handle.close()
|
||||
@@ -23,6 +23,8 @@ from __future__ import annotations
|
||||
import json
|
||||
from pathlib import Path
|
||||
|
||||
from .atomicwrite import write_atomically
|
||||
|
||||
|
||||
class MuteStore:
|
||||
"""Reads and writes the set of channels whose notifications are muted, memory-first.
|
||||
@@ -92,7 +94,4 @@ class MuteStore:
|
||||
def _save(self) -> None:
|
||||
"""Persist the whole muted set atomically (a crash mid-write keeps the old file)."""
|
||||
data = {"muted": sorted(self._state)}
|
||||
self._path.parent.mkdir(parents=True, exist_ok=True)
|
||||
tmp = self._path.with_suffix(self._path.suffix + ".tmp")
|
||||
tmp.write_text(json.dumps(data, indent=2), encoding="utf-8")
|
||||
tmp.replace(self._path)
|
||||
write_atomically(self._path, json.dumps(data, indent=2))
|
||||
@@ -42,6 +42,7 @@ from dataclasses import dataclass
|
||||
from pathlib import Path
|
||||
from typing import Any
|
||||
|
||||
from .atomicwrite import write_atomically
|
||||
from .watch_store import DEFAULT_SILENCE_HOURS, OFF, SILENCE_CHOICES_H
|
||||
|
||||
#: Display groups, in the order the page and the file present them. The order is the
|
||||
@@ -602,10 +603,7 @@ class Preferences:
|
||||
"""
|
||||
if self._path is None:
|
||||
raise RuntimeError("these preferences have no file to save to")
|
||||
self._path.parent.mkdir(parents=True, exist_ok=True)
|
||||
tmp = self._path.with_suffix(self._path.suffix + ".tmp")
|
||||
tmp.write_text(self.as_yaml(), encoding="utf-8")
|
||||
tmp.replace(self._path)
|
||||
write_atomically(self._path, self.as_yaml())
|
||||
|
||||
|
||||
#: The set installed for this process. Starts as a defaults-only, file-less instance so
|
||||
|
||||
@@ -17,6 +17,7 @@ from datetime import datetime
|
||||
from pathlib import Path
|
||||
|
||||
from .admin_store import admin_key
|
||||
from .atomicwrite import write_atomically
|
||||
from .models import Contact, utcnow
|
||||
|
||||
#: How many command-line entries are kept per node (newest last).
|
||||
@@ -109,7 +110,4 @@ class RemoteStore:
|
||||
|
||||
def _write(self, records: dict[str, dict]) -> None:
|
||||
"""Persist ``records`` atomically (crash mid-write keeps the previous file)."""
|
||||
self._path.parent.mkdir(parents=True, exist_ok=True)
|
||||
tmp = self._path.with_suffix(self._path.suffix + ".tmp")
|
||||
tmp.write_text(json.dumps(records, indent=2), encoding="utf-8")
|
||||
tmp.replace(self._path)
|
||||
write_atomically(self._path, json.dumps(records, indent=2))
|
||||
@@ -32,6 +32,7 @@ from dataclasses import dataclass
|
||||
from pathlib import Path
|
||||
from typing import TYPE_CHECKING, Any
|
||||
|
||||
from .atomicwrite import write_atomically
|
||||
from .device_config import DeviceConfigError, format_value, get_spec, parse_value
|
||||
|
||||
if TYPE_CHECKING:
|
||||
@@ -147,10 +148,7 @@ class SettingsStore:
|
||||
if values
|
||||
}
|
||||
}
|
||||
self._path.parent.mkdir(parents=True, exist_ok=True)
|
||||
tmp = self._path.with_suffix(self._path.suffix + ".tmp")
|
||||
tmp.write_text(json.dumps(data, indent=2), encoding="utf-8")
|
||||
tmp.replace(self._path)
|
||||
write_atomically(self._path, json.dumps(data, indent=2))
|
||||
|
||||
|
||||
@dataclass(frozen=True)
|
||||
|
||||
@@ -0,0 +1,110 @@
|
||||
"""Tests for the one-interactive-session-per-directory guard, and the atomic writer.
|
||||
|
||||
Both exist for the same reason: every copy of MeshTerm on a machine shares one data
|
||||
directory unless told otherwise, so two of them running at once is a thing that happens
|
||||
by accident rather than by choice.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import json
|
||||
import subprocess
|
||||
import sys
|
||||
from pathlib import Path
|
||||
|
||||
import pytest
|
||||
|
||||
from meshterm.core.atomicwrite import write_atomically
|
||||
from meshterm.core.instancelock import LOCK_NAME, InstanceBusy, hold_instance_lock
|
||||
|
||||
REPO_ROOT = Path(__file__).resolve().parent.parent
|
||||
|
||||
|
||||
def test_the_lock_is_released_when_the_holder_leaves(tmp_path: Path) -> None:
|
||||
"""Two sessions in sequence are fine; the guard is against overlap, not against reuse."""
|
||||
with hold_instance_lock(tmp_path):
|
||||
pass
|
||||
with hold_instance_lock(tmp_path):
|
||||
pass
|
||||
|
||||
|
||||
def test_the_lock_file_lives_beside_the_data(tmp_path: Path) -> None:
|
||||
"""The lock sits in the directory it guards, and the directory is made if missing."""
|
||||
target = tmp_path / "not-yet"
|
||||
with hold_instance_lock(target):
|
||||
assert (target / LOCK_NAME).exists()
|
||||
|
||||
|
||||
def test_a_second_process_is_turned_away(tmp_path: Path) -> None:
|
||||
"""The real test of an OS lock: another *process* cannot take it.
|
||||
|
||||
Nothing in-process proves anything here — a lock that only excluded the thread that
|
||||
already holds it would pass a single-process test and fail in the one situation it
|
||||
exists for.
|
||||
"""
|
||||
program = (
|
||||
"import pathlib, sys\n"
|
||||
f"sys.path.insert(0, {str(REPO_ROOT)!r})\n"
|
||||
"from meshterm.core.instancelock import hold_instance_lock, InstanceBusy\n"
|
||||
"try:\n"
|
||||
f" with hold_instance_lock(pathlib.Path({str(tmp_path)!r})):\n"
|
||||
" print('TAKEN')\n"
|
||||
"except InstanceBusy:\n"
|
||||
" print('REFUSED')\n"
|
||||
)
|
||||
with hold_instance_lock(tmp_path):
|
||||
done = subprocess.run(
|
||||
[sys.executable, "-c", program], capture_output=True, text=True, timeout=60
|
||||
)
|
||||
assert done.stdout.strip() == "REFUSED", done.stderr[-500:]
|
||||
|
||||
|
||||
def test_the_refusal_says_how_to_run_two(tmp_path: Path) -> None:
|
||||
"""The message is mostly the way out, because that is what the reader needs next.
|
||||
|
||||
Someone who sees this is doing something reasonable — trying a downloaded build, or
|
||||
running a second radio — so being told *no* without being told *how* would be the
|
||||
unhelpful half of the answer.
|
||||
"""
|
||||
message = str(InstanceBusy(tmp_path))
|
||||
assert "MESHTERM_HOME" in message
|
||||
assert str(tmp_path) in message
|
||||
|
||||
|
||||
def test_a_write_lands_whole_or_not_at_all(tmp_path: Path) -> None:
|
||||
"""The written file holds exactly what was asked for, and no scratch file survives."""
|
||||
target = tmp_path / "sub" / "contacts.json"
|
||||
write_atomically(target, json.dumps({"a": 1}))
|
||||
assert json.loads(target.read_text(encoding="utf-8")) == {"a": 1}
|
||||
assert list(tmp_path.rglob("*.tmp")) == []
|
||||
|
||||
|
||||
def test_the_scratch_file_is_named_for_its_writer(tmp_path: Path, monkeypatch) -> None:
|
||||
"""Two processes writing the same file must not share one temporary name.
|
||||
|
||||
The rename is atomic; the thing being renamed is not. A fixed ``.tmp`` name lets two
|
||||
writers interleave inside one scratch file and then each rename whatever ended up
|
||||
there over the real one.
|
||||
"""
|
||||
seen: list[Path] = []
|
||||
real = Path.write_text
|
||||
|
||||
def spy(self: Path, *args: object, **kwargs: object) -> int:
|
||||
seen.append(self)
|
||||
return real(self, *args, **kwargs) # type: ignore[arg-type]
|
||||
|
||||
monkeypatch.setattr(Path, "write_text", spy)
|
||||
write_atomically(tmp_path / "x.json", "{}")
|
||||
assert seen and str(__import__("os").getpid()) in seen[0].name
|
||||
|
||||
|
||||
def test_a_failed_write_leaves_no_litter(tmp_path: Path, monkeypatch) -> None:
|
||||
"""A write that raises cleans up after itself rather than leaving a partial file."""
|
||||
|
||||
def boom(self: Path, *args: object, **kwargs: object) -> int:
|
||||
raise OSError("disk full")
|
||||
|
||||
monkeypatch.setattr(Path, "write_text", boom)
|
||||
with pytest.raises(OSError):
|
||||
write_atomically(tmp_path / "y.json", "{}")
|
||||
assert list(tmp_path.rglob("*.tmp")) == []
|
||||
Reference in new issue
Block a user