From 20b906057d1c1df810cd0c2cfe235c27df9f3e5f Mon Sep 17 00:00:00 2001 From: Christophe Besson Date: Fri, 18 Sep 2026 16:13:53 +0200 Subject: fix(node): creating and removing a folder are syscalls too MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The last two handlers on the loop. Both were synchronous, so a member creating a folder on a root that had spun down held the node for the spin-up, exactly as a chunk read did. Where a check and an act belong together they are now one call rather than two awaits, and the single disk thread is what makes that atomic: `_mkdir_if_absent` so two members creating the same name cannot both find nothing there and have the second `mkdir` raise where a refusal was meant, and `_rmdir_if_empty` for the reason the caller already re-tested emptiness — the first test happened before a round trip to the operator's browser, and a file can land in between. Two awaits would reopen that window one size smaller. The guard is now the whole class rather than the calls that were fixed. It walks the module's syntax tree and fails on any filesystem call outside the handful of functions written to be run through `off_disk` — a new handler that stats a root inline would pass every measured test, because those exercise the handlers that exist today. Checked by putting a call back: it names the function and the line. It leaves ffmpeg's own scratch files out, listed rather than silently allowed: they are under `tempfile.mkstemp` on the system disk, not on a group root, so they are not what spins down — but they do read a whole transcode into memory from the loop, and the day that matters it is a different measurement from this one. Co-Authored-By: Claude Opus 5 --- .../src/meshbay_node/transport/webrtc_server.py | 71 ++++++++++++++++------ 1 file changed, 52 insertions(+), 19 deletions(-) (limited to 'packages/meshbay-node/src/meshbay_node/transport/webrtc_server.py') diff --git a/packages/meshbay-node/src/meshbay_node/transport/webrtc_server.py b/packages/meshbay-node/src/meshbay_node/transport/webrtc_server.py index 92f2951..3bb0df7 100644 --- a/packages/meshbay-node/src/meshbay_node/transport/webrtc_server.py +++ b/packages/meshbay-node/src/meshbay_node/transport/webrtc_server.py @@ -646,9 +646,9 @@ class WebRTCPeerSession: elif mtype == MNP.FILE_UPLOAD: self._spawn(self._do_file_upload(msg)) elif mtype == MNP.DIR_CREATE: - self._do_dir_create(msg) + self._spawn(self._do_dir_create(msg)) elif mtype == MNP.DIR_DELETE: - self._do_dir_delete(msg) + self._spawn(self._do_dir_delete(msg)) elif mtype == MNP.FILE_DELETE: self._do_file_delete(msg) elif mtype == MNP.ADMIN_RESPONSE: @@ -2034,7 +2034,7 @@ class WebRTCPeerSession: self._send(reply) self._audit_join("gek_wrapped", f"group={group_id[:8]}") - def _do_dir_create(self, msg: dict) -> None: + async def _do_dir_create(self, msg: dict) -> None: """ Create a directory, for any member of the group. @@ -2085,20 +2085,19 @@ class WebRTCPeerSession: "code": "root_unavailable"}) return - parent = safe_subdir(roots, parent_rel) - if parent is None or not parent.is_dir(): + parent = await off_disk(roots, safe_subdir, roots, parent_rel) + if parent is None or not await off_disk(roots, parent.is_dir): self._send({"type": "error", "detail": "Invalid directory"}) return - target = safe_subdir(roots, f"{parent_rel}/{name}") + target = await off_disk(roots, safe_subdir, roots, f"{parent_rel}/{name}") if target is None: self._send({"type": "error", "detail": "Invalid directory"}) return - if target.exists(): - self._send({"type": "error", "detail": "Already exists"}) + refusal = await off_disk(roots, _mkdir_if_absent, target) + if refusal is not None: + self._send({"type": "error", "detail": refusal}) return - - target.mkdir(parents=False) virtual = roots.virtual_of(target) or f"{parent_rel}/{name}" log.info("Directory created by %s: %s", self._user_id[:8], virtual) self._audit("dir_create", virtual) @@ -2113,7 +2112,7 @@ class WebRTCPeerSession: found = roots.split(rel or "") return found is not None and not found[1] - def _do_dir_delete(self, msg: dict) -> None: + async def _do_dir_delete(self, msg: dict) -> None: """ Remove an empty directory, for the node operator. @@ -2131,17 +2130,17 @@ class WebRTCPeerSession: return rel = (msg.get("dir") or "").strip("/") - target = safe_subdir(roots, rel) + target = await off_disk(roots, safe_subdir, roots, rel) # A root itself is not deletable here: removing one is a configuration # change, and doing it through a file operation would leave the group # config naming a directory nobody can reach. if target is None or self._names_a_root(roots, rel): self._send({"type": "error", "detail": "Invalid directory"}) return - if not target.is_dir(): + if not await off_disk(roots, target.is_dir): self._send({"type": "error", "detail": "Not a directory"}) return - if any(target.iterdir()): + if not await off_disk(roots, _is_empty_dir, target): self._send({"type": "error", "detail": "Directory is not empty"}) return if not self._has_admin_authority(): @@ -2157,9 +2156,9 @@ class WebRTCPeerSession: rel = pending["subject"] ctx = self._group_ctx() roots: RootSet | None = ctx.get("roots") - target = safe_subdir(roots, rel) if roots else None + target = await off_disk(roots, safe_subdir, roots, rel) if roots else None if (target is None or self._names_a_root(roots, rel) - or not target.is_dir()): + or not await off_disk(roots, target.is_dir)): self._send({"type": "error", "detail": "Not a directory"}) return @@ -2173,11 +2172,9 @@ class WebRTCPeerSession: # Checked again after the signature: the emptiness test that let this # through happened before a round trip to the operator's browser, and a # file could have landed in the meantime. - if any(target.iterdir()): + if not await off_disk(roots, _rmdir_if_empty, target): self._send({"type": "error", "detail": "Directory is not empty"}) return - - target.rmdir() log.info("Directory removed by %s: %s", self._user_id[:8], rel) self._audit("dir_delete", rel) self._send({"type": MNP.DIR_DELETE_ACK, "v": MNP_VERSION, "dir": rel}) @@ -6607,6 +6604,42 @@ class WebRTCPeerSession: await self._pc.close() +def _mkdir_if_absent(target: Path) -> str | None: + """ + Create a directory unless it is already there, or say why not. + + Both in one call, not a check awaited and then an act: the disk thread is + one worker, so nothing can slip between them. Split across two awaits, two + members creating the same name would both find nothing there and the second + `mkdir` would raise where a refusal was meant. Blocking; called through + `off_disk`. + """ + if target.exists(): + return "Already exists" + target.mkdir(parents=False) + return None + + +def _is_empty_dir(target: Path) -> bool: + """Blocking; called through `off_disk`.""" + return not any(target.iterdir()) + + +def _rmdir_if_empty(target: Path) -> bool: + """ + Remove a directory if nothing is in it. False if something is. + + The emptiness test and the removal are one call for the reason the caller + re-tests at all: the first test happened before a round trip to the + operator's browser, and a file can land in between. Two awaits here would + reopen the same window one size smaller. Blocking; called through `off_disk`. + """ + if any(target.iterdir()): + return False + target.rmdir() + return True + + def _locate(roots: RootSet, entry) -> tuple[Path | None, str | None]: """ Where an entry is, and whether it is readable — or the refusal to send. -- cgit v1.2.3