fix(assets): link shared assets atomically instead of check-then-act
Problem
rx.asset(shared=True) symlinks the asset into assets/external/ with a
check-then-act sequence that loses every race it can enter:
if not dst_file.exists() and (
not dst_file.is_symlink() or dst_file.resolve() != src_file_shared.resolve()
):
try:
dst_file.symlink_to(src_file_shared)
except FileExistsError:
# This happens when Simon builds the app on a bind mount in a docker container.
dst_file.unlink()
dst_file.symlink_to(src_file_shared)
dst_file.unlink()raisesFileNotFoundErrorwhen another process removed
the link between this one'sFileExistsErrorand itsunlink().- The retry
symlink_to()raisesFileExistsErrorwhen another process
recreated it in the same window — unhandled, so it escapes too. - The guard itself is racy, and wrong independently of concurrency:
exists()
follows the link, so a destination pointing at some other file that happens
to exist makes it skip instead of repointing.
asset() runs during page evaluation for every shared asset, so any concurrent
compile hits this. In our CI it is pytest-xdist workers each standing up an app
instance against a shared checkout: one split of four fails, the other three
pass on the same commit, and which split fails moves between runs.
FileNotFoundError: [Errno 2] No such file or directory:
'.../assets/external/reflex_components_internal/components/base/gradient_profile/GradientProfile.js'
Happened while evaluating page 'builder'
The existing comment blames docker bind mounts. That is one cause; the general
one is concurrency, with no container involved — and reading as a niche
environment quirk is why the path was left non-idempotent.
Fix
Build the link under a unique temporary name in the destination directory and
rename it into place. os.replace() is atomic on POSIX and overwrites whatever
the loser of the race left behind, so there is no unlink(), no retry, and no
window another process can invalidate:
try:
# Already correct: leave it alone so file watchers see no change.
if dst_file.readlink() == src_file:
return
except OSError:
# Missing, or not a symlink: fall through and replace it.
pass
tmp_file = dst_file.with_name(f".{dst_file.name}.{uuid.uuid4().hex}.tmp")
try:
tmp_file.symlink_to(src_file)
tmp_file.replace(dst_file)
except OSError:
tmp_file.unlink(missing_ok=True)
raise
No except was widened — errors still propagate, so asset() can never return
a path with no symlink behind it and fail later as a 404 on the built asset.
The exists()/is_symlink() guard is dropped. The replacement fast path
compares readlink() against the intended target, which keeps the "don't churn
the file watcher by re-creating a correct link" property that
remove_stale_external_asset_symlinks() cares about, while actually converging
on the right target when a concurrent writer pointed the link elsewhere.
Verification
Both reported failures reproduce deterministically before the fix, via a
monkeypatched competitor that writes to the destination around each symlink
call:
| test | old behavior |
|---|---|
test_shared_asset_survives_concurrent_removal | the reported FileNotFoundError at dst_file.unlink() |
test_shared_asset_survives_concurrent_recreation | unhandled FileExistsError from the retry |
test_shared_asset_converges_on_correct_target[symlink_to_decoy] | leaves the link pointing at the wrong file |
test_shared_asset_converges_on_correct_target[regular_file] | leaves a stale regular file in place |
test_shared_asset_is_thread_safe | (new) 8 threads x 25 calls, asserts no temp link is left behind |
Unmocked multi-process repro — 8 concurrent processes, 64 tasks x 300 compiles
into one working directory, with a competitor unlinking the destination:
29/64 workers fail before, 0/64 after.
Full unit suite passes (8130 passed, 75.86% coverage); ruff and pyright clean.
Notes for review
- Behavior change: a plain file at the destination is now replaced by a
symlink, whereexists()previously left it alone. This is the intended
convergence guarantee, and it is the proper fix for the bind-mount case the
old comment described rather than the retry. That path is generated territory. - Leftover temp links: a process killed between
symlink_toandreplace
leaves a dot-prefixed.tmplink pointing at a valid target, which
remove_stale_external_asset_symlinks()won't reap since it only collects
broken links. Two-syscall window; happy to add a sweep if reviewers want it. - Out of scope, same bug class:
remove_stale_external_asset_symlinks()is
itself check-then-act —path.unlink()anddirpath.rmdir()will raise under
the same concurrent compiles. Left for a follow-up to keep this diff local. - Windows unverified:
os.replaceusesMOVEFILE_REPLACE_EXISTINGso
replacing an existing link should work, but this was only exercised on Linux.
Symlink-privilege requirements are unchanged.