Skip to content

Fix rx.asset(shared=True) crash on Windows without Developer Mode - #7703

Open
AP24110011695 wants to merge 3 commits into
reflex-dev:mainfrom
AP24110011695:fix/shared-asset-windows-fallback
Open

AP24110011695 wants to merge 3 commits into
reflex-dev:mainfrom
AP24110011695:fix/shared-asset-windows-fallback

Conversation

@AP24110011695

@AP24110011695 AP24110011695 commented Oct 31, 2026 •

Copy link
Copy Markdown
Contributor

All Submissions:

  • Have you followed the guidelines stated in CONTRIBUTING.md file?
  • Have you checked to ensure there aren't any other open Pull Requests for the desired changed?

Type of change

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

Description

On Windows without Developer Mode (or admin rights), creating a symlink raises OSError: [WinError 1314] A required privilege is not held by the client. _link_shared_asset handles Windows rename races meticulously but not symlink-creation denial, so rx.asset(path="x.js", shared=True) — reached from any component render — crashes the whole app compile on a default Windows machine. Reproduced on Windows 10 x64, Python 3.12, current main:

OSError: [WinError 1314] A required privilege is not held by the client: '...src.js' -> '...\.e79838f0...tmp'

No GitHub issue found for this; the docs make no mention of a Windows requirement.

Fix

When symlink creation is denied, stage the link as a hardlink (shares the source content, no privileges needed on the same drive), and as a copy when that is also denied (source on another drive). The existing atomic staged-rename and retry logic is unchanged — it operates on tmp_file either way.

_links_to is extended so the idempotency guarantee survives the fallbacks: a hardlink matches by inode (Path.samefile), a copy by content digest (_content_digest). That preserves two behaviors callers rely on: a second link is a no-op so file watchers see no change, and an edited source re-copies on the next compile instead of serving stale content forever.

Tests

3 new tests in tests/units/assets/test_assets.py, all platform-independent (symlink denial is simulated by monkeypatching os.symlink, matching the file's existing competing-process pattern):

  • test_link_shared_asset_hardlink_fallback — content shared, idempotent second call (inode unchanged). Failed before this PR (OSError 1314 propagates), passes after.
  • test_link_shared_asset_copy_fallback — copy created when os.link is also denied. Failed before (same), passes after.
  • test_link_shared_asset_copy_refreshes_edited_source — edited source re-copies. Failed before, passes after.

Validated end-to-end on a real Windows machine without Developer Mode: _link_shared_asset previously raised 1314, now produces a working hardlink; 5 previously-failing tests in the file (which link through the product path) now pass locally too. The 12 remaining local failures all create symlinks inside the tests themselves and pass on Linux CI.

  • ruff check / ruff format --check clean; pyright clean.

Docs

docs/assets/overview.md alert updated with the Windows fallback behavior.

Changes To Core Features:

  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you written new tests for your core changes, as applicable?
  • Have you successfully ran tests with your changes locally?

After these steps, you're ready to open a pull request.

a. Descriptive title above.
b. Description above.
c. No linked issue — self-found via code inspection on Windows.

View guided diff

rx.asset(shared=True) crashed on Windows without Developer Mode (or admin
rights): creating the symlink raises WinError 1314 and nothing handled
it, so the whole app failed to compile with a cryptic error.

When symlink creation is denied, stage the link as a hardlink, which
shares the source content on the same drive, and as a copy when the
source lives on another drive. The content check in _links_to now also
recognizes hardlinks (same inode) and copies (same digest), so a second
link is still a no-op for file watchers, and an edited source re-copies
on the next compile instead of serving stale content.
@AP24110011695
AP24110011695 requested review from a team and Alek99 as code owners October 31, 2026 19:55

@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.

All reported issues were addressed across 4 files

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

View guided diff | Re-trigger cubic

Comment thread tests/units/assets/test_assets.py Resolved
Comment thread reflex/assets.py Resolved
Comment thread docs/assets/overview.md Resolved
Comment thread reflex/assets.py Resolved
@greptile-apps

greptile-apps Bot commented Oct 31, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerView in GreptileConfidence Score: 5/5

[Medium impact] The PR appears safe to merge; the previous stale-copy finding is fixed and no new actionable issue was found.

Summary

Makes rx.asset(shared=True) work when Windows denies symlink creation.

  • Shared assets still compile when Windows blocks symbolic links.

AP24110011695 deferred cleanup of assets from removed modules to #7705 because fallback files need source-tracking information. Greptile accepted that follow-up.

Reviews (3) · Last reviewed commit: "fix(assets): compare copied assets by co..." · Reviewed by Greptile

Comment thread reflex/assets.py Resolved
Comment thread reflex/assets.py Resolved
Address the review findings on PR reflex-dev#7703:

- A read-only source made every fallback path break later compiles: a
  hardlink shares the read-only bit, and copy2 mirrors it, and Windows
  refuses to replace a read-only destination. Read-only sources now go
  straight to a copy, which is made writable so the next compile can
  replace it. Regression test included.
- The copy check hashed both files in full on every compile; compare
  size and mtime (copy2 preserves both) instead, so an unchanged copy
  costs no file reads while an edited source still re-copies.
- Document the copy fallback as 'when hardlinking is unavailable'
  (cross-drive, FAT32) rather than cross-drive only.
- Assert an unchanged copy is left alone in the copy fallback test,
  mirroring the hardlink test.
Comment thread reflex/assets.py Resolved

@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.

All reported issues were addressed across 4 files (changes from recent commits).

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

View guided diff | Re-trigger cubic

Comment thread reflex/assets.py Resolved
…path

Address the re-review findings on PR reflex-dev#7703: size and mtime are not
content identity - a same-size rewrite that preserves the timestamp
(archive restore, timestamp-preserving tools) passed as unchanged and
kept serving stale bytes. Equal sizes are now compared by digest; the
size pre-check keeps the edited-source case on stats alone.
@masenf masenf added the documentation Improvements or additions to documentation label Nov 2, 2026
@FarhanAliRaza FarhanAliRaza added the bug Something isn't working label Nov 12, 2026 — with ChatGPT Codex Connector
@codspeed

codspeed Bot commented Nov 12, 2026

Copy link
Copy Markdown

Merging this PR will regress 1 benchmark

3 warnings

⚠️ 17 benchmarks are dominated by system calls

System calls cannot be simulated consistently and are left out of the measure. The reported value understates the real cost. Switch to the Walltime instrument to measure them.

Learn more about system calls

See affected benchmarks

⚠️ 119 benchmarks with a significant change ran on different hardware

Their base and head results were measured on machines with a different CPU, core count or memory, so the changes might not come from the code. Run base and head on the same runner type.

Learn more about Macro Runners

See affected benchmarks

⚠️ 119 benchmarks with a significant change ran in a different software environment

Their base and head results were measured with a different OS or toolchain, such as compiler, runtime or library versions, so the changes might come from this difference rather than from the code. Open an affected benchmark to see what changed.

See affected benchmarks

⚡ 118 improved benchmarks
❌ 1 regressed benchmark
✅ 29 untouched benchmarks

Warning

Please fix the performance issues or acknowledge them on CodSpeed.

Performance Changes

Benchmark BASE HEAD Efficiency
❌ test_route_matching[100] 48.2 µs 50.7 µs -5.01%
⚡ test_state_update_wire_serialization[mapping_100] 284.9 µs 256.3 µs +11.16%
⚡ test_isinstance_container[list_optional] 288.4 ms 261.6 ms +10.23%
⚡ test_state_update_wire_serialization[dataclass_1000] 15.8 ms 14.3 ms +10.13%
⚡ test_route_argument_extraction 115.4 µs 105.8 µs +9.04%
⚡ test_decode_event_headers[64] 182.3 µs 167.7 µs +8.69%
⚡ test_computed_dependency_fanout[50] 1.8 ms 1.6 ms +8.57%
⚡ test_process_event_burst_same_token[100] 1.3 s 1.2 s +8.57%
⚡ test_event_payload_plain_collection 69.4 ms 64 ms +8.55%
⚡ test_set_state[redis] 33.6 ms 31.1 ms +8.02%
⚡ test_import_reflex 9.7 ms 9 ms +7.97%
⚡ test_modify_state[redis] 75 ms 69.5 ms +7.97%
⚡ test_match_operation 22 ms 20.4 ms +7.82%
⚡ test_isinstance_container[list_int] 112.6 ms 104.5 ms +7.73%
⚡ test_isinstance_scalar 107.1 ms 99.5 ms +7.66%
⚡ test_process_event_burst_independent_tokens[100] 173.7 ms 161.5 ms +7.6%
⚡ test_state_collection_assignment[1000] 11.4 ms 10.6 ms +7.5%
⚡ test_decode_event_headers[4] 41.4 µs 38.5 µs +7.48%
⚡ test_get_state_uncached[disk] 55.7 ms 51.8 ms +7.47%
⚡ test_hydration_snapshot[200] 8.9 ms 8.3 ms +7.37%
... ... ... ... ...

ℹ️ Only the first 20 benchmarks are displayed. Go to the app to view all benchmarks.

Tip

Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.


Comparing AP24110011695:fix/shared-asset-windows-fallback (90ff27f) with main (aeea15a)

Open in CodSpeed

@AP24110011695

AP24110011695 commented Nov 12, 2026 •

Copy link
Copy Markdown
Contributor Author

The ubuntu 3.12 unit-tests failure is a timing-threshold flake unrelated to the change: test_state_manager_lock_warning_threshold_contend asserted no lock-held warning while the contended lock took 1.503s on the runner — a load-dependent threshold in the redis state-manager tests, not in asset handling. Every asset-related job (windows 3.12/3.14/3.15, ubuntu 3.11/3.13/3.14/3.15) passed.

@AP24110011695

AP24110011695 commented Nov 12, 2026 •

Copy link
Copy Markdown
Contributor Author

Same read on this report: test_route_matching[100] (−5.01%) is unrelated to the asset-fallback diff, and the warnings show base and head ran on different hardware and different software environments — the deltas aren't attributable to the code (118 improved / 1 regressed).

This branch has not been deployed

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

Labels

bug Something isn't working documentation Improvements or additions to documentation windows

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants