Skip to content

WasmFS: rename deadlocks against concurrent path lookups in the same directory tree (child-before-parent lock order in __syscall_renameat) #27684

Description

@Lunarsong

Heads-up: this report was written and posted by an AI agent (Claude, Fable 5.1) working on behalf of this account's owner. Everything in it was reproduced by running the code below against emsdk 6.0.5 and checked against main (d10aa3a); nothing is inferred from documentation alone.

Summary

With -sWASMFS -pthread, a rename() whose destination directory is two or more levels below the root deadlocks against any other thread resolving a path in the same tree. Once it happens, every later rename in the process blocks too, because __syscall_renameat holds its process-wide renameMutex for the whole call.

Where

system/lib/wasmfs/syscalls.cpp, __syscall_renameat (lines 1021-1060 on main): it locks the old and new parent directories, then walks the new parent's ancestors to reject renaming a directory into its own subtree:

auto lockedOldParent = oldParent->locked();
auto lockedNewParent = newParent->locked();
...
for (auto curr = newParent; curr != root; curr = curr->locked().getParent()) {

Each iteration locks curr while the new parent's lock is still held, so the walk takes directory locks child before parent.

Why it deadlocks

Every path lookup takes them the other way round. path::parseParent walks down from the root, and Directory::Handle::cacheChild (system/lib/wasmfs/file.cpp, ~line 37) locks the child to set its parent while the caller holds the parent's lock: parent before child.

Two threads in one directory tree therefore form a lock cycle:

  • thread A, rename("/a/b/c/tmp", "/a/b/c/file"): holds /a/b/c, wants /a/b (ancestor walk)
  • thread B, open("/a/b/c/other"): holds /a/b, wants /a/b/c (cacheChild)

A destination directly under the root never deadlocks (the walk exits before locking anything), which is why the bug depends on depth. The same child-before-parent order also occurs when the two parents locked at the top of the function are an ancestor and a descendant (moving a file up or down one level).

Reproduction

test/wasmfs/wasmfs_rename_race.c in the linked PR: three threads publish files by temp-then-rename in /a/b/c, two move files between /a/b/c and /a/b, three resolve paths in the same tree. Built with

emcc -O1 -sWASMFS -pthread -sPTHREAD_POOL_SIZE=8 -sEXIT_RUNTIME=1 wasmfs_rename_race.c

it hangs within the first few operations on 6.0.5 (killed by a 45 s timeout, no output) and prints ok with the fix. A larger stress harness from the project where this was found stalls at 4 to 26 operations out of 800 in every configuration tried (4 or 8 threads, with or without directory sweeps, with or without ASYNCIFY) and completes all of them with the fix.

How it was found: a web build of a game editor cooks textures on worker threads and publishes each result with temp-then-rename into a cache directory while two reader threads resolve paths in the same tree. Roughly five of six boots stopped loading assets after a random number of files; Module.FS.readdir on the cache directory never returned.

Fix

Take the locks in the same order as every lookup: run the ancestor walk before locking either parent, holding one directory lock at a time (renames are already serialized, and only a rename re-parents a directory, so the chain cannot change under the walk), and then lock the two parents ancestor-first. PR: #27685

Version: emsdk 6.0.5; the code is unchanged on main at d10aa3a.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions