Skip to content

Fix race condition in stateful pages marker with atomic writes - #7142

Open
masenf wants to merge 2 commits into
mainfrom
claude/upbeat-brown-jglset
Open

masenf wants to merge 2 commits into
mainfrom
claude/upbeat-brown-jglset

Conversation

@masenf

@masenf masenf commented Sep 14, 2026

Copy link
Copy Markdown
Collaborator

Type of change

  • Bug fix (non-breaking change which fixes an issue)

Description

Fixes a startup race condition in backend-only mode with multiple workers where a worker could read a truncated .web/backend/stateful_pages.json file and crash with JSONDecodeError.

Root cause: The marker file was written directly, so concurrent writers could produce partial/corrupted JSON that readers would encounter.

Solution: Write the marker atomically by:

  1. Writing to a temporary file in the same directory
  2. Using Path.replace() to atomically swap it into place
  3. Ensuring readers only ever see a complete, valid marker (or no marker at all)

Additionally, refactored the marker reading logic to handle the case where the marker doesn't exist yet (another worker may be writing it), which correctly falls through to evaluating all pages rather than assuming "no marker" means "no stateful pages".

Changes

reflex/app.py:

  • Modified _write_stateful_pages_marker() to use atomic writes via tempfile.mkstemp() and Path.replace()
  • Always writes the marker (even for stateless apps with an empty list), ensuring backend workers have a definitive signal

reflex/compiler/compiler.py:

  • Extracted marker reading into _read_stateful_pages_marker() helper that returns None if the marker doesn't exist yet
  • Simplified compile_app() logic: only skip full page evaluation if the marker exists and is readable; missing marker falls through to normal compilation

Tests:

  • Added test_write_stateful_pages_marker_never_truncates_final_path(): verifies the marker is never opened for writing (only swapped into place)
  • Added test_write_stateful_pages_marker_is_always_written(): verifies stateless apps write an empty marker
  • Added test_write_stateful_pages_marker_concurrent_readers_see_valid_json(): stress test with 4 concurrent writers and 4 concurrent readers, ensuring no JSONDecodeError or partial reads
  • Updated test_compile_registers_plugin_routes_on_backend_early_return() to correctly expect all pages evaluated when marker is missing

Test Plan

All new unit tests pass and cover the atomic write behavior and concurrent access patterns. Existing tests updated to reflect the corrected behavior when the marker is absent.

https://claude.ai/code/session_01GSjqov3yBj4cBasJqzyrrQ

Review in cubic

In prod backend-only mode with several Granian workers, every worker
takes the "evaluate all pages" path when .web is absent and writes
.web/backend/stateful_pages.json with mode "w", truncating it. A worker
starting slightly later saw the backend dir, read an empty or partial
marker, and died with JSONDecodeError.

Write the marker to a temporary file in the same directory and swap it
into place with Path.replace so readers only ever see a complete file.
Read the marker with a single read_text call and treat FileNotFoundError
as "no marker yet", falling through to evaluating all pages, which also
closes the window between one worker creating the backend dir and
swapping its marker in. The marker is now always written, including for
stateless apps, so that fall-through does not slow their startup.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GSjqov3yBj4cBasJqzyrrQ
@masenf
masenf requested a review from a team as a code owner September 14, 2026 20:50
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GSjqov3yBj4cBasJqzyrrQ
@greptile-apps

greptile-apps Bot commented Sep 14, 2026

Copy link
Copy Markdown
Contributor

RetriggerConfidence Score: 2/5

The PR is not yet safe to merge because backend startup can fail on legacy malformed markers or cross-account artifacts, and dry-run compilation now mutates generated state.

Findings

  1. P1 Marker permissions become restrictive
  2. P1 Legacy corruption still crashes
  3. P1 Dry runs modify artifacts

Summary

This PR changes the stateful-pages marker to be written through a same-directory temporary file and atomically replaced, and makes backend-only compilation evaluate every page when the marker is absent.

  • Adds concurrent reader/writer and stateless-marker regression coverage.
  • Preserves the missing-marker fallback for workers that start before another worker publishes the marker.
  • Still needs handling for legacy malformed markers, dry-run filesystem semantics, and the restrictive permissions inherited from mkstemp().

Reviews (1) · Last reviewed commit: "Rename news fragment to PR number"

Comment thread reflex/app.py
Comment on lines +1730 to +1735
fd, tmp_path = tempfile.mkstemp(
dir=stateful_pages_marker.parent,
prefix=f"{stateful_pages_marker.name}.",
suffix=".tmp",
)
tmp_marker = Path(tmp_path)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Marker permissions become restrictive

If frontend compilation and backend startup run under different Unix accounts while sharing generated backend artifacts, this marker may become unreadable. mkstemp() creates the temporary file with mode 0600, and replace() preserves that mode on the final path. The backend account will then get PermissionError when reading the marker, whereas the previous writer normally created a umask-filtered file readable by other accounts.

Knowledge Base Used: Application runtime

Comment on lines +1219 to +1223
marker = prerequisites.get_backend_dir() / constants.Dirs.STATEFUL_PAGES
try:
return json.loads(marker.read_text())
except FileNotFoundError:
return None

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Legacy corruption still crashes

A deployment can retain a truncated marker produced by the previous direct-write implementation, but this reader handles only a missing file. On the first backend-only startup after upgrading, json.loads() still raises JSONDecodeError, causing the same startup crash instead of evaluating all pages and regenerating the marker.

Knowledge Base Used:

Comment thread reflex/app.py
Comment on lines +1726 to +1739
stateful_pages_marker = (
prerequisites.get_backend_dir() / constants.Dirs.STATEFUL_PAGES
)
stateful_pages_marker.parent.mkdir(parents=True, exist_ok=True)
fd, tmp_path = tempfile.mkstemp(
dir=stateful_pages_marker.parent,
prefix=f"{stateful_pages_marker.name}.",
suffix=".tmp",
)
tmp_marker = Path(tmp_path)
try:
with os.fdopen(fd, "w") as f:
json.dump(list(self._stateful_pages), f)
tmp_marker.replace(stateful_pages_marker)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Dry runs modify artifacts

Removing the state check makes stateless dry-run compilation create or replace this marker. The compiler calls this method before its dry-run return, so reflex compile --dry can mutate .web/backend/stateful_pages.json and overwrite an existing marker even though the command promises not to make changes.

Knowledge Base Used: Frontend compilation pipeline

@codspeed-hq

codspeed-hq Bot commented Sep 14, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 40 untouched benchmarks
⏩ 8 skipped benchmarks1


Comparing claude/upbeat-brown-jglset (fcef7f4) with main (f4acb86)

Open in CodSpeed

Footnotes

  1. 8 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

4 issues found and verified against the latest diff

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="tests/units/test_app.py">

<violation number="1" location="tests/units/test_app.py:4854">
P2: In `test_write_stateful_pages_marker_concurrent_readers_see_valid_json`, exceptions raised inside the writer threads are silently lost. `writer()` has no try/except, so if `_write_stateful_pages_marker()` raises (e.g., an `OSError` during `mkstemp`/`replace`, or a future regression in the write path), that thread dies via the default threading excepthook and pytest never observes it — the test keeps running and can still pass by reading the last good marker written by the remaining writers. The final `marker.read_text()` assert only fails if every writer died before creating the marker at all, so partial failures go undetected. The reader threads have the same blind spot: they only record `AssertionError`/`JSONDecodeError`, so any other exception in `read_text` also terminates a reader silently. Since this test is the regression guard for the exact race being fixed, capture exceptions from both writers and readers into `errors` so the test fails whenever the write path misbehaves.</violation>
</file>

<file name="reflex/app.py">

<violation number="1" location="reflex/app.py:1730">
P2: When frontend and backend run under different Unix accounts, this replacement leaves the marker readable only by the compiling account. Set the temporary file's mode to the marker's intended shared-readable mode before `replace()`.</violation>

<violation number="2" location="reflex/app.py:1737">
P2: If `os.fdopen` fails before taking ownership of the `mkstemp` descriptor, the exception cleanup removes the temporary path but leaks the descriptor. Close `fd` before opening `tmp_marker` by path, matching the existing atomic-writer pattern in `reflex/compiler/utils.py`.</violation>
</file>

<file name="reflex/compiler/compiler.py">

<violation number="1" location="reflex/compiler/compiler.py:1222">
P1: Catch `json.JSONDecodeError` as well as `FileNotFoundError` so a truncated marker from an older deployment falls through to full page evaluation instead of crashing backend-only startup.</violation>
</file>

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

marker = prerequisites.get_backend_dir() / constants.Dirs.STATEFUL_PAGES
try:
return json.loads(marker.read_text())
except FileNotFoundError:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1: Catch json.JSONDecodeError as well as FileNotFoundError so a truncated marker from an older deployment falls through to full page evaluation instead of crashing backend-only startup.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At reflex/compiler/compiler.py, line 1222:

<comment>Catch `json.JSONDecodeError` as well as `FileNotFoundError` so a truncated marker from an older deployment falls through to full page evaluation instead of crashing backend-only startup.</comment>

<file context>
@@ -1206,6 +1206,23 @@ def _register_plugin_routes(app: App, plugins: Sequence[Plugin]) -> None:
+    marker = prerequisites.get_backend_dir() / constants.Dirs.STATEFUL_PAGES
+    try:
+        return json.loads(marker.read_text())
+    except FileNotFoundError:
+        return None
+
</file context>
Suggested change
except FileNotFoundError:
except (FileNotFoundError, json.JSONDecodeError):

Comment thread tests/units/test_app.py
stop = threading.Event()
errors: list[BaseException] = []

def writer():

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: In test_write_stateful_pages_marker_concurrent_readers_see_valid_json, exceptions raised inside the writer threads are silently lost. writer() has no try/except, so if _write_stateful_pages_marker() raises (e.g., an OSError during mkstemp/replace, or a future regression in the write path), that thread dies via the default threading excepthook and pytest never observes it — the test keeps running and can still pass by reading the last good marker written by the remaining writers. The final marker.read_text() assert only fails if every writer died before creating the marker at all, so partial failures go undetected. The reader threads have the same blind spot: they only record AssertionError/JSONDecodeError, so any other exception in read_text also terminates a reader silently. Since this test is the regression guard for the exact race being fixed, capture exceptions from both writers and readers into errors so the test fails whenever the write path misbehaves.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At tests/units/test_app.py, line 4854:

<comment>In `test_write_stateful_pages_marker_concurrent_readers_see_valid_json`, exceptions raised inside the writer threads are silently lost. `writer()` has no try/except, so if `_write_stateful_pages_marker()` raises (e.g., an `OSError` during `mkstemp`/`replace`, or a future regression in the write path), that thread dies via the default threading excepthook and pytest never observes it — the test keeps running and can still pass by reading the last good marker written by the remaining writers. The final `marker.read_text()` assert only fails if every writer died before creating the marker at all, so partial failures go undetected. The reader threads have the same blind spot: they only record `AssertionError`/`JSONDecodeError`, so any other exception in `read_text` also terminates a reader silently. Since this test is the regression guard for the exact race being fixed, capture exceptions from both writers and readers into `errors` so the test fails whenever the write path misbehaves.</comment>

<file context>
@@ -4799,3 +4800,82 @@ def test_compile_emits_stage_spans(
+    stop = threading.Event()
+    errors: list[BaseException] = []
+
+    def writer():
+        for _ in range(50):
+            app._write_stateful_pages_marker()
</file context>

Comment thread reflex/app.py
)
tmp_marker = Path(tmp_path)
try:
with os.fdopen(fd, "w") as f:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: If os.fdopen fails before taking ownership of the mkstemp descriptor, the exception cleanup removes the temporary path but leaks the descriptor. Close fd before opening tmp_marker by path, matching the existing atomic-writer pattern in reflex/compiler/utils.py.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At reflex/app.py, line 1737:

<comment>If `os.fdopen` fails before taking ownership of the `mkstemp` descriptor, the exception cleanup removes the temporary path but leaks the descriptor. Close `fd` before opening `tmp_marker` by path, matching the existing atomic-writer pattern in `reflex/compiler/utils.py`.</comment>

<file context>
@@ -1714,14 +1717,29 @@ def _compile(
+        )
+        tmp_marker = Path(tmp_path)
+        try:
+            with os.fdopen(fd, "w") as f:
                 json.dump(list(self._stateful_pages), f)
+            tmp_marker.replace(stateful_pages_marker)
</file context>
Suggested change
with os.fdopen(fd, "w") as f:
os.close(fd)
with tmp_marker.open("w") as f:

Comment thread reflex/app.py
Comment on lines +1730 to +1734
fd, tmp_path = tempfile.mkstemp(
dir=stateful_pages_marker.parent,
prefix=f"{stateful_pages_marker.name}.",
suffix=".tmp",
)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: When frontend and backend run under different Unix accounts, this replacement leaves the marker readable only by the compiling account. Set the temporary file's mode to the marker's intended shared-readable mode before replace().

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At reflex/app.py, line 1730:

<comment>When frontend and backend run under different Unix accounts, this replacement leaves the marker readable only by the compiling account. Set the temporary file's mode to the marker's intended shared-readable mode before `replace()`.</comment>

<file context>
@@ -1714,14 +1717,29 @@ def _compile(
+            prerequisites.get_backend_dir() / constants.Dirs.STATEFUL_PAGES
+        )
+        stateful_pages_marker.parent.mkdir(parents=True, exist_ok=True)
+        fd, tmp_path = tempfile.mkstemp(
+            dir=stateful_pages_marker.parent,
+            prefix=f"{stateful_pages_marker.name}.",
</file context>
Suggested change
fd, tmp_path = tempfile.mkstemp(
dir=stateful_pages_marker.parent,
prefix=f"{stateful_pages_marker.name}.",
suffix=".tmp",
)
fd, tmp_path = tempfile.mkstemp(
dir=stateful_pages_marker.parent,
prefix=f"{stateful_pages_marker.name}.",
suffix=".tmp",
)
os.chmod(tmp_path, 0o644)

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants