Skip to content

Commit 368c60f

Browse files
committed
fix(app-tools): harden the dev lock against cross-process races
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) Co-Authored-By: Riff
1 parent 00f085d commit 368c60f

7 files changed

Lines changed: 472 additions & 48 deletions

File tree

.changeset/dev-server-lock.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'@modern-js/app-tools': patch
3+
---
4+
5+
feat: add a project operation lock: `dev`/`start` register a shared lock and `build`/`deploy` hold an exclusive lock, so a second `modern dev` (or a build during dev) fails fast with the running instance's URL, PID and kill command instead of silently switching ports or clobbering `dist`; opt in to multiple dev servers with `modern dev --allow-multiple`

packages/solutions/app-tools/src/commands/dev.ts

Lines changed: 27 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import {
1010
type Alias,
1111
DEFAULT_DEV_HOST,
1212
SERVER_DIR,
13+
getAddressUrls,
1314
getMeta,
1415
logger,
1516
} from '@modern-js/utils';
@@ -100,6 +101,28 @@ export const dev = async (
100101

101102
const host = normalizedConfig.dev?.host || DEFAULT_DEV_HOST;
102103

104+
// Same protocol/host derivation as `prettyInstructions`, so the URLs in
105+
// the lock file match what the terminal prints (HTTPS, custom host, IPv6).
106+
const markLockReady = () => {
107+
const isHttps = Boolean(
108+
normalizedConfig.dev?.https ||
109+
(normalizedConfig.tools as { devServer?: { https?: unknown } })
110+
?.devServer?.https,
111+
);
112+
markDevLockReady(appDirectory, metaName, {
113+
port,
114+
host,
115+
urls:
116+
typeof port === 'number'
117+
? getAddressUrls(
118+
isHttps ? 'https' : 'http',
119+
port,
120+
normalizedConfig.dev?.host,
121+
).map(({ url }) => url)
122+
: undefined,
123+
});
124+
};
125+
103126
if (apiOnly) {
104127
const { server } = await createDevServer(
105128
{
@@ -115,10 +138,7 @@ export const dev = async (
115138
host,
116139
},
117140
() => {
118-
markDevLockReady(appContext.appDirectory, appContext.metaName, {
119-
port,
120-
host,
121-
});
141+
markLockReady();
122142
printInstructions(
123143
hooks,
124144
appContext,
@@ -144,14 +164,13 @@ export const dev = async (
144164
async (err?: Error) => {
145165
if (err) {
146166
logger.error('Occur error %s, when start dev server', err);
167+
// Never mark the lock ready for a server that failed to listen.
168+
return;
147169
}
148170

149171
logger.debug('listen dev server done');
150172

151-
markDevLockReady(appContext.appDirectory, appContext.metaName, {
152-
port,
153-
host,
154-
});
173+
markLockReady();
155174

156175
await afterListen();
157176
},

packages/solutions/app-tools/src/plugins/devLock.ts

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import { getArgv, getCommand, minimist } from '@modern-js/utils';
22
import type { AppTools, CliPlugin } from '../types';
33
import {
44
acquireCommandLock,
5+
getDevLockIntent,
56
normalizeLockOperation,
67
releaseAllLocks,
78
suspendForRestart,
@@ -30,17 +31,23 @@ export default (): CliPlugin<AppTools> => ({
3031
return;
3132
}
3233

33-
// The flag is consumed here, before Commander parses the command
34-
// action options (which happens after `onPrepare`).
35-
const args = minimist(getArgv(), {
36-
boolean: ['allow-multiple'],
37-
});
34+
// The run entry (`createRunOptions`) parses `--allow-multiple` /
35+
// `RunOptions.allowMultiple` and stores a run-scoped intent; raw argv
36+
// is only a fallback for callers that drive `cli.init()` directly.
37+
const intent = getDevLockIntent(appDirectory);
38+
const allowMultiple =
39+
intent?.allowMultiple ??
40+
Boolean(
41+
minimist(getArgv(), { boolean: ['allow-multiple'] })[
42+
'allow-multiple'
43+
],
44+
);
3845

3946
await acquireCommandLock({
4047
appDirectory,
4148
metaName,
4249
operation,
43-
allowMultiple: operation === 'dev' && Boolean(args['allow-multiple']),
50+
allowMultiple: operation === 'dev' && allowMultiple,
4451
});
4552
});
4653

packages/solutions/app-tools/src/run/index.ts

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import { handleSetupResult } from '../compat/hooks';
66
import {
77
type DevServerLockError,
88
isDevServerLockError,
9+
setDevLockIntent,
910
} from '../utils/devLock';
1011
import { getConfigFile } from '../utils/getConfigFile';
1112
import { loadInternalPlugins } from '../utils/loadPlugins';
@@ -18,6 +19,12 @@ export interface RunOptions {
1819
internalPlugins?: InternalPlugins;
1920
initialLog?: string;
2021
version: string;
22+
/**
23+
* Intentionally run this dev server alongside an already-running one
24+
* (equivalent to the `--allow-multiple` CLI flag; the option wins over
25+
* argv when both are present).
26+
*/
27+
allowMultiple?: boolean;
2128
}
2229
export async function createRunOptions({
2330
cwd,
@@ -26,6 +33,7 @@ export async function createRunOptions({
2633
version,
2734
internalPlugins,
2835
configFile,
36+
allowMultiple,
2937
}: RunOptions) {
3038
const nodeVersion = process.versions.node;
3139
const versionArr = nodeVersion.split('.').map(Number);
@@ -57,7 +65,8 @@ export async function createRunOptions({
5765
const cliParams = minimist<{
5866
c?: string;
5967
config?: string;
60-
}>(process.argv.slice(2));
68+
'allow-multiple'?: boolean;
69+
}>(process.argv.slice(2), { boolean: ['allow-multiple'] });
6170
/**
6271
* Commands that support specify config files
6372
* `new` command need to use `--config-file` params,because `--config` is already used
@@ -88,6 +97,13 @@ export async function createRunOptions({
8897

8998
const plugins = await loadInternalPlugins(appDirectory, internalPlugins);
9099

100+
// Single place where the multi-dev intent is resolved: the typed run
101+
// option wins over the raw `--allow-multiple` flag. Stored run-scoped
102+
// (keyed by appDirectory) for the dev-lock plugin to read in `onPrepare`.
103+
setDevLockIntent(appDirectory, {
104+
allowMultiple: allowMultiple ?? Boolean(cliParams['allow-multiple']),
105+
});
106+
91107
return {
92108
cwd,
93109
initialLog: initialLog || `Modern.js Framework v${version}`,

0 commit comments

Comments
 (0)