emscripten-core / emscripten-core/emscripten

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

Open
#27,684 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
C++
Stars
27.6k
Forks
3.6k
Avg merge
1d 1h
Merged PRs (30d)
105

Description

> 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` (d10aa3af1b); 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:

```cpp
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: https://github.com/emscripten-core/emscripten/pull/27685

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

Contributor guide

Open the contributing guide

Research direction

Start in system/lib/wasmfs/syscalls.cpp at __syscall_renameat, then read path::parseParent and Directory::Handle::cacheChild in system/lib/wasmfs/file.cpp to compare lock ordering. Run test/wasmfs/wasmfs_rename_race.c with the documented pthread build; done means it prints ok without hanging during concurrent path lookups and renames.

Written by the indexing model from the issue text.

Assessment

Tech stack
cpp, wasm
Domain
operating-systems
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Clearly specified
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.