feat(app-tools): add dev server lock file - #8809
Open
keepview wants to merge 15 commits into
Open
Conversation
🦋 Changeset detectedLatest commit: e4264a3 The changes in this PR will be included in the next version bump. This PR includes changesets to release 120 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
✅ Deploy Preview for modernjs-byted ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Contributor
Rsdoctor Bundle Diff Analysis📊 Quick Summary
Generated by Rsdoctor GitHub Action |
keepview
force-pushed
the
feat/dev-server-lock
branch
from
August 11, 2026 07:34
368c60f to
62fe2fa
Compare
keepview
changed the title
keepview
force-pushed
the
feat/dev-server-lock
branch
from
August 13, 2026 12:22
cdb72ed to
5015395
Compare
…uild conflicts dev/start register a shared lock; build and deploy (including --skip-build) hold an exclusive lock for their full duration. Every command checks the lock registry inside a cross-process mutex before any directory cleanup, so a conflicting start fails fast with an actionable error (URL, PID, kill command) instead of silently switching ports or clobbering dist. Run another dev server intentionally with `modern dev --allow-multiple`.
Review follow-ups on the initial implementation: - registry mutex now publishes a fully-populated candidate directory and atomically renames it into place, so there is no created-but-unwritten owner window; dead-owner takeover is an atomic rename to a tombstone named after the dead owner's token, so concurrent waiters cannot displace the next holder, and a live holder is never broken - `--allow-multiple` is resolved once at the run entry (typed `RunOptions.allowMultiple` wins over argv) and handed to the guard through a run-scoped, appDirectory-keyed intent - the ready lock records the real listen URLs via `getAddressUrls` (HTTPS / custom host / IPv6 consistent with the terminal output) and is never marked ready when `listen` reports an error - add the missing changeset and multi-process mutex tests (crash between create and rename, concurrent takeover, live-holder wait, exclusive sections never overlapping across real processes)
…dev lock - never garbage-collect token tombstones: a waiter suspended for an arbitrary time may still act on a pre-suspension read of a dead owner, and the tombstone occupying that target name is what stops it from renaming the current holder's mutex away - make the multi-open intent truly invocation-scoped: it is cleared in run()'s finally, so a stale allowMultiple can never leak into a later cli.init(); hot restarts re-derive the privilege from the process's own lock file instead (persisted allowMultiple + self re-entry) - only remove ownerless mutex candidates when the creator pid embedded in the candidate name is provably dead, so a process paused between mkdir and the owner write is left alone - fix the test fixtures to record real process start times on macOS and build @modern-js/app-tools in build:required so the multi-process mutex suite actually runs (and hard-fails on a missing artifact) in CI
- a failed TCP probe no longer deletes a lock whose holder's identity (pid + real start time) is verified alive: during a config hot restart the server closes before CLI init re-runs, and probe-based deletion in that window let a concurrent build wipe a live dev's output; the probe is now only the fallback signal on platforms without a start-time source - surface real IO errors (read-only cache dir, ENOSPC) from mutex candidate preparation immediately instead of spinning in the retry loop, and route the vanished-mutex path through the normal deadline/backoff instead of a hot continue - restore run()'s rejection semantics: the typed DevServerLockError is printed once, then re-thrown; the CLI bin catches it and exits, while programmatic callers of the public ./cli/run entry get a rejection again instead of a silent resolve - only suggest kill/taskkill when the holder's identity is verified; otherwise ask the user to verify the pid first
A live pid now always keeps its lock — the port probe is removed entirely instead of remaining a deletion signal on platforms without a start-time source. On Windows the previous fallback still deleted a live dev's lock during the hot-restart window (server closed before CLI init re-runs) and let a concurrent build wipe its output; the worst case now is a conservative block on a reused pid that asks the user to verify, never a false green light. Tests assert the hot-restart window blocks builds on every platform, with identityVerified reported as false where no start-time source exists.
…ilures The dev server lock made an existing hazard visible: i18n/app-ssr and i18n/routes-ssr each run two test files against the same project directory, so under file-level parallelism a dev server and a build were racing over one dist/ all along. Every affected file now runs against its own copy via a shared createIsolatedTestApp helper (standardizing what pure-esm-project already did). node_modules is linked entry by entry instead of as a whole so each copy gets its own .cache lock registry and .modern-js directory — a whole-dir link would recreate the conflict. Test utils stop masking real errors: a dev server that exits before becoming ready rejects with its full output instead of resolving undefined, modernBuild throws on a non-zero exit unless allowFailure is set, killApp(undefined) returns instead of tree-killing undefined.pid, and killApp now waits for the process to actually exit so a follow-up build cannot race the dying dev server's lock cleanup. A new dev-server-lock fixture adds the real-CLI regression: build is rejected (EBUILD_BLOCKED_BY_DEV) while dev holds the lock and succeeds after it stops; a second dev is rejected (EDEV_SERVER_RUNNING); isolated copies run dev and build in parallel.
With modernBuild no longer swallowing non-zero exits and dev startup failures no longer resolving undefined, three pre-existing issues became visible (all reproduce with main's app-tools as well): - pure-esm-project: the two test files' isolated copies shared one lock registry and .modern-js through a whole-directory node_modules symlink; they now use the shared createIsolatedTestApp helper with per-entry links - server-config: the fixture's server code had two type errors that failed 'Running ts compile' on every build - compatibility: the fixture asserted an es5 build without stating any browserslist while the framework default targets modern engines, and the router runtime's dynamic import() has no es5 form; the fixture now pins ie>=11 and excludes that one chunk from the syntax check
Document project command concurrency in the English and Chinese command references. Cover the build --watch lifecycle with a real CLI integration test.
The section is cross-cutting (dev/start/build/deploy), so it now follows the last command instead of sitting between start and build; the build section links to it. Also state explicitly that --allow-multiple is only accepted by dev/start — build/deploy reject it as an unknown option, since an exclusive lock has nothing to relax.
A second dev started as 'start --allow-multiple' (the alias shares the dev command's options) must boot alongside the first, while build in the same directory stays rejected — the flag only relaxes dev-dev exclusion.
`releaseRegistryMutex` deleted the shared `.mutex` path recursively, which removes `owner.json` first and the directory last. A waiter can rename its complete candidate onto the momentarily empty directory in that window, so the trailing rmdir fails with `ENOTEMPTY` — surfacing as an untyped error out of `acquireCommandLock` — and takes files from the new owner with it. The macOS unit job caught it as `blocked ENOTEMPTY` in the multi-process suite. Release now requires a readable owner whose token matches, renames `.mutex` to a token-private `.mutex-release-<token>` path, and deletes that instead. The rename is atomic: either the whole directory leaves the shared name or nothing is touched. Deleting the detached copy is best-effort — the mutex is already released at that point — and `cleanMutexDebris` collects any copy left behind by a crash, which is safe because a detached mutex is no longer reachable by any waiter. A missing or unreadable owner is now also a no-op: it means the mutex is mid-handover, and nothing may be deleted on the strength of a guess. Co-Authored-By: Riff
Sweeping `.mutex-release-*` ran outside the per-entry guard, so a directory that could not be removed turned into a raw filesystem error thrown out of the acquisition that happened to trigger the sweep. The sweep is best-effort: whatever it cannot collect is left for the next one. The release regression tests now pin the mechanism rather than its end state: `renameSync` must move the shared `.mutex` to the token-private path and `rmSync` must never be handed the shared path, which the previous recursive delete cannot satisfy. A second test drives a failing sweep and asserts the acquire still completes. Co-Authored-By: Riff
Implements the reviewed concurrency semantics on top of the project lock: - conflict detection now compares write sets — the generated-files directory (output.tempDir) and the build output directory (output.distPath). Sharing either still conflicts; commands with both directories distinct (the official per-config isolation) run in parallel, whatever they are. Locks from older writers without these fields conservatively span the whole app - exclusive commands that conflict with a finite build/deploy/inspect no longer fail: they queue, re-entering the registry mutex every round to re-clean stale locks and re-decide atomically, log who they are waiting for (first wait, then periodically), hold nothing while sleeping, and fail fast if a persistent task takes the project first. No in-framework deadline — CI's overall timeout is the backstop, and FIFO order among multiple waiters is not promised - locks carry a generic persistent attribute: dev servers and build --watch are persistent and can never be queued behind; a programmatic watch build upgrades its lock in the build action - inspect participates as a short exclusive holder, since it recreates internalDirectory on startup and dumps configs into dist
keepview
force-pushed
the
feat/dev-server-lock
branch
from
August 17, 2026 09:35
a81b88a to
e4264a3
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Running
modern devtwice used to silently switch ports, and abuild/deploystarted during dev wipeddist/under the running server. Both now get a deterministic outcome — fail fast, queue, or run in parallel — decided before any directory cleanup.output.tempDir) and the output directory (output.distPath). Sharing either conflicts; commands with both distinct (per-config isolation) run in parallel — whatever they are.build --watch) or starting a second bare dev fails fast with the holder's URL, PID and kill command; conflicting finite builds (build/deploy/inspect) queue automatically — re-entering the cross-process mutex each round, logging who they wait for, promising no FIFO order and no in-framework deadline.inspectholds a short exclusive lock (it recreatesinternalDirectoryon startup);servestays lock-free.node_modules/.cache/<metaName>/locks/v1/, are checked before cleanup inside a cross-process mutex, and stale locks (crash,kill -9) are cleaned automatically; a live pid never loses its lock.modern dev --allow-multiple.Tests
startalias, legacy locks without write-set fields.tests/integration/dev-server-lock, 7 cases) including two same-directory builds queueing to success andinspectrejected during dev; fulltests/integrationgreen (92 files, 381 passed).