Repository navigation
fix(macos-native): 并发打开返回 Busy、Store 目录 0700、透传 Rust/git 环境(#534) - #570
jiangkoumo wants to merge 8 commits into
Conversation
…ive commands On native macOS (host_process), workspace.exec sets HOME to the Sandbox's scratch directory (<sandbox>/tmp), so commands do not write into the user's home directory. Pass RUSTUP_HOME (~/.rustup), CARGO_HOME (~/.cargo) and GIT_CONFIG_GLOBAL (~/.gitconfig, else $XDG_CONFIG_HOME/git/config or ~/.config/git/config) from the caller's environment or real HOME when present, before applying request.environment so explicit values win. Docker and namespaces backends are unchanged. Tests: local_search_path::a_native_command_receives_existing_user_toolchain_and_git_locations covers absent paths, XDG and ~/.gitconfig resolution, ~/.rustup and ~/.cargo passthrough, and caller environment / request environment overrides while HOME remains <sandbox>/tmp. Production LOC: 137010 -> 137039 (delta +29) Scope: tools/production_loc.py --root on parent d430474 and committed tree (core 38400 -> 38400, engine 62799 -> 62799, connect 15947 -> 15976 (+29), api 19864 -> 19864, dsh 0 -> 0; reference absent (retired R9c; last counted 65,417 at tree 498dd19)).
…r permission errors When the Store directory does not exist, kernel_guard::check creates it as 0700 (without changing permissions on existing user directories). When it already exists with non-private mode (mode & 0o077 != 0), wrong owner or non-directory type, kernel_guard::check refuses up front during store.create, project.init and StoreService::open_direct with the directory path, current mode, expected mode 0700 and chmod command. In layerfs-daemon's config::private_directory, include the directory path and which check failed (type, uid or mode with chmod hint). Document the private 0700 Store directory requirement on native macOS. Production LOC: 137010 -> 137111 (delta +101) Scope: tools/production_loc.py, combined core+engine+connect+api (reference absent (retired R9c; last counted 65,417 at tree 498dd19)). core 38400 -> 38400 (0); engine 62799 -> 62899 (+100); connect 15947 -> 15947 (0); api 19864 -> 19865 (+1).
…nd Busy open refusal
On native macOS, multiple processes open the same SQLite Store directly on APFS/HFS+. Previously, Writer::acquire was a no-op on macOS (retaining an outdated comment from when it locked the main database file instead of the parent directory), and Session::connect ran initial WAL header recovery without holding the cooperative directory lock. Concurrent Store opens collided on SQLite WAL_WRITE_LOCK with busy_timeout=0 and failed with Store open: Store busy before effect wrapped as Failed instead of Busy.
- writer_admission.rs: enable blocking exclusive flock on the Store's parent directory vnode on macOS (matching Linux). The parent directory is a distinct vnode from SQLite's main/WAL/SHM files, so Darwin flock never conflicts with SQLite fcntl locks and creates no extra files in the Store directory.
- connection.rs: hold Writer::acquire across writable Session::connect so concurrent first openers serialize initial WAL-index recovery (-shm initialization) with open-time write transactions.
- store_service.rs, layerfs-control store/{service,postgres}.rs: add OpenRefusal::Busy and map StorageError::Busy during Store open to Busy refusal code instead of Failed, keeping other open errors as Failed.
- Tests: enable cross-process/multi-handle writer_admission tests on macOS and add 3-process concurrent open + reservation/commit tests in writer_admission.rs and kernel_guard.rs.
Deleted tests: none.
Production LOC: 137010 -> 137025 (delta +15)
Core 38400 -> 38400 (+0); engine 62799 -> 62810 (+11: layerfs-persistence 12314 -> 12317, layerfs-store-service 10771 -> 10779); connect 15947 -> 15947 (+0); api 19864 -> 19868 (+4: layerfs-control 17528 -> 17532); dsh 0 -> 0 (+0).
reference absent (retired R9c; last counted 65,417 at tree 498dd19).
Method: python3 -B tools/production_loc.py --detail on exact parent d430474 and the committed tree.
…er flock Updates the 2.0 architecture guide pages to reflect macOS parent-directory writer admission flock, writable Session::connect lock acquisition, and OpenRefusal::Busy on Store open. Deleted tests: none. Production LOC: 137025 -> 137025 (delta +0) Core 38400 -> 38400 (+0); engine 62810 -> 62810 (+0); connect 15947 -> 15947 (+0); api 19868 -> 19868 (+0); dsh 0 -> 0 (+0). reference absent (retired R9c; last counted 65,417 at tree 498dd19). Method: python3 -B tools/production_loc.py --detail on exact parent bb0dd0b and the committed tree.
Merges Store directory 0700 validation and clear daemon permission refusals (first-parent comparison against 7beb47c). Deleted tests: none. Production LOC: 137025 -> 137126 (delta +101) Core 38400 -> 38400 (+0); engine 62810 -> 62910 (+100: layerfs-daemon 7312 -> 7337, layerfs-store-service 10779 -> 10854); connect 15947 -> 15947 (+0); api 19868 -> 19869 (+1: layerfs-control 17532 -> 17533); dsh 0 -> 0 (+0). reference absent (retired R9c; last counted 65,417 at tree 498dd19). Method: python3 -B tools/production_loc.py --detail on first parent 7beb47c and the committed tree.
Merges native Sandbox RUSTUP_HOME, CARGO_HOME and GIT_CONFIG_GLOBAL passthrough and resolves the sandbox.md table cell conflict by keeping both native macOS requirements in the same cell (first-parent comparison against ac0c037). Deleted tests: none. Production LOC: 137126 -> 137155 (delta +29) Core 38400 -> 38400 (+0); engine 62910 -> 62910 (+0); connect 15947 -> 15976 (+29: layerfs-sandbox 8599 -> 8628); api 19869 -> 19869 (+0); dsh 0 -> 0 (+0). reference absent (retired R9c; last counted 65,417 at tree 498dd19). Method: python3 -B tools/production_loc.py --detail on first parent ac0c037 and the committed tree.
…e user_environment on macOS Addresses the Ephemeral-AI-Lab#534 review findings after merging fix/534a-native-macos, fix/534b-native-macos and fix/534c-native-macos: - engine/crates/layerfs-store-service/tests/support/installed_store.rs: create Fixture::built_with and Fixture::cloned directories with mode 0700 so kernel_guard tests added in 534A pass with 534B's private-directory enforcement. - engine/crates/layerfs-store-service/src/kernel_guard.rs: make kernel_guard::check purely side-effect-free (never creating directories) and always take store.parent() as the Store directory without a .sqlite filename heuristic. - api/crates/layerfs-control/src/store/identity.rs, engine/crates/layerfs-store-service/src/install.rs, connect/crates/layerfs-store-backup/src/backup/restore.rs: create missing Store directories as 0700 in the actual creation/installation entrypoints (make_empty, install_local after installer_user(), and backup::restore::fetch after installer_user()), then run kernel_guard::check(destination) on the Store file path and clean up newly created directories if the guard fails. - connect/crates/layerfs-sandbox/src/backend/host_process/exec.rs: gate user_environment() with #[cfg(target_os = "macos")] (returning empty off macOS) so Linux host_process command environments remain unchanged. - api/crates/layerfs-api/tests/{project_family,store_open_cache}.rs, engine/crates/layerfs-store-service/tests/kernel_guard.rs: verify side-effect-free missing-directory refusal, project.init 0755 refusal and macOS running-call writer lock protection. Deleted tests: none. Production LOC: 137155 -> 137153 (delta -2) Core 38400 -> 38400 (+0); engine 62910 -> 62887 (-23: layerfs-store-service 10854 -> 10831); connect 15976 -> 15992 (+16: layerfs-sandbox 8628 -> 8635, layerfs-store-backup 3227 -> 3236); api 19869 -> 19874 (+5: layerfs-control 17533 -> 17538); dsh 0 -> 0 (+0). reference absent (retired R9c; last counted 65,417 at tree 498dd19). Method: python3 -B tools/production_loc.py --detail on exact parent 61b644f and the committed tree.
There was a problem hiding this comment.
Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota. Try requesting a review at a lower effort level.
An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing.
|
Retested 6be2f6c on my M3 Max (macOS 26.6.2), release build,
The full flow passes too: init, fork, mount, edits from the host, commit, cargo build, commit with target/, fork, warm build (0.54 s), merge, close. Nothing is left mounted or running after close. Also on the base, so it was there before the flock: with 5 or more agents some commits wait 1 to 4.3 s while the other agents' commits keep overtaking them (up to 58 during one wait), sometimes until every agent that wasn't stuck has finished. Is that known, or should I open an issue for it? |
doctor accepted 0701 on store/<name> and suggested chmod g-rwx,o-rw, but the daemon refuses any group/other bit there. Check the Store folder with 0o077 and suggest chmod 700; other home folders keep 0701.
@ImmuneFOMO 非常感谢这么细的复测,17 轮、1716 次 commit 加上 kill -9 和杀 daemon 的场景,比我们自己测得还全。
再次感谢! |
|
Checked 8a85fa4 on my Mac. The doctor tests pass (5 of 5), and for the 0701 Store folder doctor now says |
|
Hey @jiangkoumo, PR #570 is currently conflicting with recent layerfs_v2_0_0 updates. Since this touches macOS native concurrent open & permissions, could you rebase onto the latest base when convenient? Thanks! |
Writer::acquire returned an empty lock on macOS, so every writer of the shared Store file ran with SQLite's busy_timeout=0 and met another process's write window as Busy (Ephemeral-AI-Lab#534, Ephemeral-AI-Lab#647). Widen the lane's cfg to macOS: acquire and release now flock the Store's parent directory there, an independent description that leaves SQLite's own main/WAL/SHM descriptors and fcntl locks untouched. Port the same admission to the two points that write outside a transaction. A writable Session takes the lane for its open: the PRAGMA and schema statements of an open write, and another opener now waits for it instead of failing. And the idle WAL checkpoint's try_acquire takes the same non-blocking branch on macOS as on Linux (LockExclusiveNonblock, EWOULDBLOCK -> Ok(None)), because the port must never turn that skip into a blocking wait while the caller holds its Session turn; Ephemeral-AI-Lab#570 merged into a base with this function would have blocked there. A Store open refused for Busy now answers OpenRefusal::Busy (SDK code Busy) instead of Failed, since nothing was attempted. Tests: writer_admission's cross-process and open-time cases and idle_checkpoint's held-lane case run on macOS too (cfg widened; the child-side socket is made blocking again, as Ephemeral-AI-Lab#570 does), and three_processes_open_and_reserve_concurrently is new: three children open the same Store at once and make twenty reservations each, all succeeding. Deleted tests: none. Production LOC: 157027 -> 157039 (delta +12) Scope: first-party core/engine/control/API (core 39787, engine 73177 -> 73188, control 38513 -> 38514, api 5550, dsh 0; reference absent, retired R9c, last counted 65,417 at tree 498dd19). Method: tools/production_loc.py at parent 1c0a6fd vs committed tree (engine/crates/layerfs-persistence/src/backend/sqlite/lane/writer_admission.rs -5, engine/crates/layerfs-persistence/src/backend/sqlite/session/connection.rs +8, engine/crates/layerfs-store-service/src/store_service.rs +8, control/crates/layerfs-store/src/service.rs +1; tests excluded).
Ports the flock and Busy-open wording of the Ephemeral-AI-Lab#534 review fix (Ephemeral-AI-Lab#570) to the current tree, without its 0700 directory, kernel-guard and SDK changes: - docs/architecture/engine/store/README.md: the cross-process writer row names the Store parent directory flock for Linux and macOS; the writer_admission.rs test bullet covers macOS child processes, concurrent opens and reservations, and keeps Cancelled on a signal Linux-only, as the test itself is. - docs/architecture/engine/store/backends.md: Writer::acquire step, the one-writer rule and the Cancelled row say Linux and macOS; the paragraph explains the separate descriptor, writable Session::connect taking the lane, and that only a writer bypassing the lane meets SQLite's Busy. - docs/architecture/engine/store/sqlite.md: Session::connect takes the writer lock when writable and releases it after profile::check; the lane holds the directory lock on both platforms. - docs/architecture/engine/store/lifecycle.md: the idle checkpoint's try_acquire lane and the closing paragraph say Linux and macOS. - docs/architecture/engine/store-calls/service.md: open_direct reports Busy("Store open: ..") before any effect, else Failed("Store open: .."). - Comments only in lane.rs, session/checkpoint.rs and records.rs: the publication lane, the skipped idle checkpoint and Cancelled are Linux and macOS, not Linux alone. Seal is unchanged: Session::seal takes the lane under cfg(target_os = "linux") and the lifecycle page keeps saying so. Deleted tests: none. Production LOC: 157052 -> 157052 (delta +0) Core 39787 -> 39787 (+0); engine 73201 -> 73201 (+0); control 38514 -> 38514 (+0); api 5550 -> 5550 (+0); reference 0 -> 0 (+0); dsh 0 -> 0 (+0). reference absent (retired R9c; last counted 65,417 at tree 498dd19). Method: python3 -B tools/production_loc.py --detail on exact parent 1df9185 and the staged tree.
|
本 PR 的写入准入(并发打开返回 Busy)已移植进 #647,并补了 digest 拒绝报确定 Busy 与 #540 的占槽修复,统一在 #647 推进和验证。这里先关闭,后续讨论请移步 #647。感谢 @ImmuneFOMO 之前的复测。 |
…phemeral-AI-Lab#570 P1) `backup.restore` installed through `home::private_dir`, which keeps an existing folder's mode: a `store/<name>` left at 0755 by an earlier restore was accepted and stayed 0755, so the daemon, `branch.fork` and `sandbox.create` refused the Store the restore had just written. The restore now runs `store.create`'s own kernel guard on an existing `store/<name>` before it stages or writes anything, and refuses with the same `Unsupported` code and `chmod 700` remedy; an absent folder is still made 0700 by the install. Tests: the refusal with nothing staged, the same folder at 0700 and an absent one (backup_refusals); a restore into a pre-made 0700 folder against MinIO (backup_family). No test deleted. Production LOC: 159347 -> 159351 (delta +4) Scope: first-party product implementation under core/engine/control/api crates' src plus shipped runtime SQL; tests, docs, tools and manifests are excluded. Counted with `tools/production_loc.py --root <tree>`, once on the parent tree 2647fc8 and once on this commit's tree; control 39452 -> 39456, api 5613 -> 5613. Migration subtotals: reference absent (retired R9c; last counted 65,417 at tree 498dd19).
…phemeral-AI-Lab#570 P1) `backup.restore`'s folder check runs when the call starts, but the download between it and the install is a long window: a `store/<name>` that appeared meanwhile, or was changed to 0755, was still installed into and stayed 0755, so the daemon, `branch.fork` and `sandbox.create` refused the Store the restore had just written (Ephemeral-AI-Lab#663 review P2-1). The restore's install step is now `backup::restore::install`, which runs the same kernel guard on the target folder again under the home lock, before it links the Store into it or renames it over the old file, and refuses with the same `Unsupported` code and `chmod 700` remedy. What was staged is dropped with the staging folder as before. The first check stays: a folder that is already not private is refused before anything is staged. Tests: `layerfs-store/tests/restore_install.rs` covers the install boundary itself, without a bucket or install permission: a folder made 0755 during the download is refused with nothing installed and its mode kept; a replaced Store's folder is refused before the replacement's own checks; an absent folder is made 0700 and the Store and record are written. The two positive cases in `backup_refusals` now assert a `Failed` from inside the download (the installer-user check as a Linux non-root user, the dead endpoint's own request with install permission) and the staging folder made and removed, instead of "not Unsupported" (review P3-1). No test deleted. Production LOC: 159351 -> 159375 (delta +24) Scope: first-party product implementation under core/engine/control/api crates' src plus shipped runtime SQL; tests, docs, tools and manifests are excluded. Counted with `tools/production_loc.py --root <tree>`, once on the parent tree 7c54540 and once on this commit's staged tree; control 39456 -> 39480, api 5613 -> 5613. Migration subtotals: reference absent (retired R9c; last counted 65,417 at tree 498dd19).
Brings the current upstream branch (af2ad92, Ephemeral-AI-Lab#658 and what it merged since be317c0) into the Ephemeral-AI-Lab#570 P1 Store-folder branch; no textual conflict. Production LOC: 159977 -> 160145 (delta +168) Scope and method: first-parent comparison (parent af2ad92, this merge's committed tree). First-party production Rust/SQL/JavaScript in core, engine, control and api; tests, fixtures, tools, docs, manifests/lockfiles and generated artifacts excluded. Counted with tools/production_loc.py --detail on each exported tree: engine 74419 -> 74500 (+81), control 39795 -> 39882 (+87); core 39787 and api 5976 unchanged. Reference absent (retired R9c; last counted 65,417 at tree 498dd19).
Ephemeral-AI-Lab#570 P1 review) Review r663.md, points 1-3 and the smaller ones: - Point 1, the merge with Ephemeral-AI-Lab#647: its `installed::Fixture` made the Store's folder with plain `std::fs::create_dir` (0755), so its new `open_direct` tests reached this branch's guard's `Unsupported` before Busy, corrupt or concurrent (`tests/support/installed_store.rs:48,119`). Both constructors now make that folder `0700` through one `private_directory`. Verified by merging `origin/fix/578-sync-volume` into a throwaway worktree: before, `kernel_guard`'s two new tests failed with `Unsupported(... has mode 0775 ...)`; after, all 5 passed. Whichever PR lands second needs this folder to be `0700`. - Point 2: the restore's install-entry check from 82eb09f is unchanged. `install` checks the target's folder before `private_dir`, `hard_link` and `rename`, under the caller's home lock (`control/crates/layerfs-store/src/backup/restore.rs:166`). - Smaller: the claim step (a `0700` `mkdir`, ownership only when this call's own `mkdir` succeeded) is now one product function, `install::make_private_directory`, used by `prepare` and by `store.create`'s `make_empty`. This removes the second copy of the ownership rule and lets an unprivileged test drive both branches and the failed `mkdir`; the `install_directory` guard cases still need the daemons' user. - Smaller: doctor now reports a folder that is a symlink as a link, and its remedy no longer `chmod`s through the link into the target (the 0777 it reported was the link's own mode). A `store/<name>` link is scanned too, so a Store folder the daemon refuses is reported rather than skipped. - Smaller: docs say the macOS `HostProcess` Rust and Git homes are the caller's own writable ones, so a native command can change the host's toolchain. - Not split: the env passthrough (a80592c) stays in this PR; the review calls the split optional. - Point 3 (the breaking change in the PR description) is in the handoff report, which no call or code change can carry. Production LOC: 160145 -> 160194 (delta +49) Scope and method: first-party production Rust/SQL/JavaScript in core, engine, control and api; tests, fixtures, tools, docs, manifests/lockfiles and generated artifacts excluded. Counted with tools/production_loc.py --detail on the exported parent tree (this commit's parent 3f039a7) and this tree: engine 74500 -> 74506 (+6), control 39882 -> 39925 (+43); core 39787 and api 5976 unchanged. Reference absent (retired R9c; last counted 65,417 at tree 498dd19).
…AI-Lab#570 P1 review R2) When `store_names` began enumerating symlinks under `home/store`, `store()` reported the symlink finding in `store_folder()` but still descended into `record.json` and `project/`. A symlink pointing to a regular file caused `paths::names(&projects)` to fail with ENOTDIR, aborting `scan()` and discarding the entire doctor report. `store_folder()` now returns whether the path is a symlink so `store()` stops descending after reporting it and continues scanning remaining Stores; `store_names()` also sorts entries for deterministic output. Added regression tests for a regular-file symlink and a dangling symlink under `home/store` alongside another Store finding. True first-parent Production LOC disclosure for merge commit 3f039a7 (history preserved without rewriting; 3f039a7's own message erroneously compared against second parent af2ad92, 159977 -> 160145, delta +168): 82eb09f -> 3f039a7: 159375 -> 160145 (delta +770); core 39787 -> 39787 (0), engine 74495 -> 74500 (+5), control 39480 -> 39882 (+402), api 5613 -> 5976 (+363); reference absent (retired R9c; last counted 65,417 at tree 498dd19). Production LOC: 160194 -> 160198 (delta +4) Scope and method: first-parent comparison (parent affb55b, this commit's tree). First-party production Rust/SQL/JavaScript/Python in core, engine, control and api; tests, fixtures, tools, docs, manifests/lockfiles and generated artifacts excluded. Counted with tools/production_loc.py --detail on exported parent tree affb55b and committed tree: control 39925 -> 39929 (+4); core 39787, engine 74506 and api 5976 unchanged. Reference absent (retired R9c; last counted 65,417 at tree 498dd19).
…reclaim only own directories (#663) Store folders are made 0700 under the daemon's rule, and doctor reports a Store folder with group or other permission bits. Every Store opener (SDK Store calls, project add, store create, backup restore) refuses a folder that is not a real 0700 directory of this user with Unsupported and a chmod 700 remedy. macOS native commands pass through the caller's RUSTUP_HOME, CARGO_HOME and GIT_CONFIG_GLOBAL. install::prepare reclaims only a directory this call's own mkdir made (AlreadyExists or an installer check failure removes nothing; a guard refusal uses a non-recursive remove_dir); restore reuses it and re-checks the target folder under the home lock before the install; store create reclaims only when make_empty fails. Breaking: an existing Store in a 0755 or 0701 folder now fails Store calls with Unsupported until chmod 700. Tests: new install_directory (engine), create_directory and restore_install (control); fixtures made 0700. Docs: store/create.md, backup-and-restore.md, doctor.md, control/backups/README.md, api/sdk/backup.md, store-calls/README.md. Durable: NOT_RUN — disabled by owner until explicit reauthorization. Production LOC: 160242 -> 160463 (delta +221); scope core 39787, engine 74521 -> 74608, control 39958 -> 40092, api 5976; reference absent (retired R9c; last counted 65,417 at tree 498dd19); method tools/production_loc.py against layerfs_v2_0_0 e8fa0a7 and the merged tree. Refs #570, #534
关联 #534(macOS native 上的三个问题),基于
layerfs_v2_0_0(d43047413),按 docs/architecture 与 #533 的 2.0 意图修,只改 2.0 保留的入口。改了什么
StoreService::open_direct遇到锁占用时返回Busy,不再统一包成Failed。store create(identity::make_empty)、install_local、backup restore在用户检查之后以 0700 创建 Store 目录;kernel_guard::check保持纯检查、无副作用(只读打开不留空目录),统一取 Store 文件的 parent。已有目录为 0755 时,project init提前报Unsupported,信息含路径、当前/期望权限与chmod 700 <path>。校验失败只清理本次新建的目录。host_process在 macOS 上透传RUSTUP_HOME、CARGO_HOME、GIT_CONFIG_GLOBAL(存在时),HOME仍为<sandbox>/tmp;用cfg(target_os = "macos")门控,Linux 行为与基线一致。验证
cargo fmt --check、tools/check_product_boundary.py、工具测试、7 个 crateclippy -D warnings、受影响测试全部通过;store_open_cache的运行中写锁测试在 macOS 上启用并通过。clippy --all-targets -D warnings通过;layerfs-store-service136、layerfs-store-backup19、layerfs-sandbox101、layerfs-control12、layerfs-api83 通过。layerfs-control --test kernel_application、layerfs-api --test workspace_names两个。已知未改
layerfs doctor对 Store 目录权限用0o076判定(允许 0701),比kernel_guard的0o077略宽,未在本 PR 修改。