Skip to content

Commit 21ee4c4

Browse files
KamirusKamirus
authored
feat(cli): add per-canister build lock to prevent parallel clobbering (#472)
## Problem Parallel `mops build` invocations targeting the same canister race on writing `.wasm`, `.did`, and `.most` output files, which can produce corrupted or mixed artifacts. ## Root cause The build loop writes to `{outputDir}/{canisterName}.wasm` (and `.did`, `.most`) with no inter-process coordination. Two concurrent builds of the same canister interleave writes to the same files. ## Fix Add per-canister file locking using `proper-lockfile`: - Before compiling each canister, acquire a lock on a sentinel file `.{canisterName}.buildlock` inside the output directory. - The second build retries with backoff until the first finishes (up to ~60 retries, 500ms–5s intervals). - The lock is released in a `finally` block on the normal path, and via a synchronous `process.on('exit')` handler when `cliError()` calls `process.exit()` (proper-lockfile's built-in signal-exit cleanup proved unreliable in this scenario). - Different canisters use separate sentinel files, so `mops build canisterA` and `mops build canisterB` can still run concurrently. With exponential backoff from 500ms to 5s over 60 retries, the total max wait is roughly 4-5 minutes — intentionally aligned with the stale timeout. The idea: if the lock holder is alive, we wait. If it's dead, the stale detection kicks in at ~5 minutes and we take over. Either way, we don't wait forever. ## Test plan - [x] New test: two parallel `mops build foo` via `Promise.all` — both succeed - [x] Existing tests pass (error paths, custom output dir, CLI flags, managed flag warnings) - [x] Pre-commit hooks pass (prettier, eslint, tsc)
1 parent 91b415f commit 21ee4c4

5 files changed

Lines changed: 192 additions & 92 deletions

File tree

‎cli/commands/build.ts‎

Lines changed: 127 additions & 92 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import { execa } from "execa";
33
import { exists } from "fs-extra";
44
import { mkdir, readFile, writeFile } from "node:fs/promises";
55
import { join } from "node:path";
6+
import { lock, unlockSync } from "proper-lockfile";
67
import { cliError } from "../error.js";
78
import { isCandidCompatible } from "../helpers/is-candid-compatible.js";
89
import { resolveCanisterConfigs } from "../helpers/resolve-canisters.js";
@@ -70,115 +71,149 @@ export async function build(
7071
motokoPath = resolveConfigPath(motokoPath);
7172
const wasmPath = join(outputDir, `${canisterName}.wasm`);
7273
const mostPath = join(outputDir, `${canisterName}.most`);
73-
let args = [
74-
"-c",
75-
"--idl",
76-
"--stable-types",
77-
"-o",
78-
wasmPath,
79-
motokoPath,
80-
...(await sourcesArgs()).flat(),
81-
...getGlobalMocArgs(config),
82-
];
83-
args.push(
84-
...collectExtraArgs(config, canister, canisterName, options.extraArgs),
85-
);
86-
87-
const isPublicCandid = true; // always true for now to reduce corner cases
88-
const candidVisibility = isPublicCandid ? "icp:public" : "icp:private";
89-
if (isPublicCandid) {
90-
args.push("--public-metadata", "candid:service");
91-
args.push("--public-metadata", "candid:args");
74+
75+
// per-canister lock to prevent parallel builds of the same canister from clobbering output files
76+
const lockTarget = join(outputDir, `.${canisterName}.buildlock`);
77+
await writeFile(lockTarget, "", { flag: "a" });
78+
79+
let release: (() => Promise<void>) | undefined;
80+
try {
81+
release = await lock(lockTarget, {
82+
stale: 300_000,
83+
retries: { retries: 60, minTimeout: 500, maxTimeout: 5_000 },
84+
});
85+
} catch {
86+
cliError(
87+
`Failed to acquire build lock for canister ${canisterName} — another build may be stuck`,
88+
);
9289
}
90+
91+
// proper-lockfile registers its own signal-exit handler, but it doesn't reliably
92+
// fire on process.exit(). This manual handler covers that gap. Double-unlock is
93+
// harmless (the second call throws and is caught).
94+
const exitCleanup = () => {
95+
try {
96+
unlockSync(lockTarget);
97+
} catch {}
98+
};
99+
process.on("exit", exitCleanup);
100+
93101
try {
94-
if (options.verbose) {
95-
console.log(chalk.gray(mocPath, JSON.stringify(args)));
102+
let args = [
103+
"-c",
104+
"--idl",
105+
"--stable-types",
106+
"-o",
107+
wasmPath,
108+
motokoPath,
109+
...(await sourcesArgs()).flat(),
110+
...getGlobalMocArgs(config),
111+
];
112+
args.push(
113+
...collectExtraArgs(config, canister, canisterName, options.extraArgs),
114+
);
115+
116+
const isPublicCandid = true; // always true for now to reduce corner cases
117+
const candidVisibility = isPublicCandid ? "icp:public" : "icp:private";
118+
if (isPublicCandid) {
119+
args.push("--public-metadata", "candid:service");
120+
args.push("--public-metadata", "candid:args");
96121
}
97-
const result = await execa(mocPath, args, {
98-
stdio: options.verbose ? "inherit" : "pipe",
99-
reject: false,
100-
});
122+
try {
123+
if (options.verbose) {
124+
console.log(chalk.gray(mocPath, JSON.stringify(args)));
125+
}
126+
const result = await execa(mocPath, args, {
127+
stdio: options.verbose ? "inherit" : "pipe",
128+
reject: false,
129+
});
101130

102-
if (result.exitCode !== 0) {
103-
if (!options.verbose) {
104-
if (result.stderr) {
105-
console.error(chalk.red(result.stderr));
106-
}
107-
if (result.stdout?.trim()) {
108-
console.error(chalk.yellow("Build output:"));
109-
console.error(result.stdout);
131+
if (result.exitCode !== 0) {
132+
if (!options.verbose) {
133+
if (result.stderr) {
134+
console.error(chalk.red(result.stderr));
135+
}
136+
if (result.stdout?.trim()) {
137+
console.error(chalk.yellow("Build output:"));
138+
console.error(result.stdout);
139+
}
110140
}
141+
cliError(
142+
`Build failed for canister ${canisterName} (exit code: ${result.exitCode})`,
143+
);
111144
}
112-
cliError(
113-
`Build failed for canister ${canisterName} (exit code: ${result.exitCode})`,
114-
);
115-
}
116-
117-
if (options.verbose && result.stdout && result.stdout.trim()) {
118-
console.log(result.stdout);
119-
}
120145

121-
options.verbose &&
122-
console.log(chalk.gray(`Stable types written to ${mostPath}`));
146+
if (options.verbose && result.stdout && result.stdout.trim()) {
147+
console.log(result.stdout);
148+
}
123149

124-
const generatedDidPath = join(outputDir, `${canisterName}.did`);
125-
const resolvedCandidPath = canister.candid
126-
? resolveConfigPath(canister.candid)
127-
: null;
150+
options.verbose &&
151+
console.log(chalk.gray(`Stable types written to ${mostPath}`));
128152

129-
if (resolvedCandidPath) {
130-
try {
131-
const compatible = await isCandidCompatible(
132-
generatedDidPath,
133-
resolvedCandidPath,
134-
);
153+
const generatedDidPath = join(outputDir, `${canisterName}.did`);
154+
const resolvedCandidPath = canister.candid
155+
? resolveConfigPath(canister.candid)
156+
: null;
135157

136-
if (!compatible) {
137-
cliError(
138-
`Candid compatibility check failed for canister ${canisterName}`,
158+
if (resolvedCandidPath) {
159+
try {
160+
const compatible = await isCandidCompatible(
161+
generatedDidPath,
162+
resolvedCandidPath,
139163
);
140-
}
141164

142-
if (options.verbose) {
143-
console.log(
144-
chalk.gray(
145-
`Candid compatibility check passed for canister ${canisterName}`,
146-
),
165+
if (!compatible) {
166+
cliError(
167+
`Candid compatibility check failed for canister ${canisterName}`,
168+
);
169+
}
170+
171+
if (options.verbose) {
172+
console.log(
173+
chalk.gray(
174+
`Candid compatibility check passed for canister ${canisterName}`,
175+
),
176+
);
177+
}
178+
} catch (err: any) {
179+
cliError(
180+
`Error during Candid compatibility check for canister ${canisterName}${err?.message ? `\n${err.message}` : ""}`,
147181
);
148182
}
149-
} catch (err: any) {
150-
cliError(
151-
`Error during Candid compatibility check for canister ${canisterName}${err?.message ? `\n${err.message}` : ""}`,
152-
);
153183
}
154-
}
155184

156-
options.verbose &&
157-
console.log(chalk.gray(`Adding metadata to ${wasmPath}`));
158-
const candidPath = resolvedCandidPath ?? generatedDidPath;
159-
const candidText = await readFile(candidPath, "utf-8");
160-
const customSections: CustomSection[] = [
161-
{ name: `${candidVisibility} candid:service`, data: candidText },
162-
];
163-
if (canister.initArg) {
164-
customSections.push({
165-
name: `${candidVisibility} candid:args`,
166-
data: canister.initArg,
167-
});
168-
}
169-
const wasmBytes = await readFile(wasmPath);
170-
const newWasm = getWasmBindings().add_custom_sections(
171-
wasmBytes,
172-
customSections,
173-
);
174-
await writeFile(wasmPath, newWasm);
175-
} catch (err: any) {
176-
if (err.message?.includes("Build failed for canister")) {
177-
throw err;
185+
options.verbose &&
186+
console.log(chalk.gray(`Adding metadata to ${wasmPath}`));
187+
const candidPath = resolvedCandidPath ?? generatedDidPath;
188+
const candidText = await readFile(candidPath, "utf-8");
189+
const customSections: CustomSection[] = [
190+
{ name: `${candidVisibility} candid:service`, data: candidText },
191+
];
192+
if (canister.initArg) {
193+
customSections.push({
194+
name: `${candidVisibility} candid:args`,
195+
data: canister.initArg,
196+
});
197+
}
198+
const wasmBytes = await readFile(wasmPath);
199+
const newWasm = getWasmBindings().add_custom_sections(
200+
wasmBytes,
201+
customSections,
202+
);
203+
await writeFile(wasmPath, newWasm);
204+
} catch (err: any) {
205+
if (err.message?.includes("Build failed for canister")) {
206+
throw err;
207+
}
208+
cliError(
209+
`Error while compiling canister ${canisterName}${err?.message ? `\n${err.message}` : ""}`,
210+
);
178211
}
179-
cliError(
180-
`Error while compiling canister ${canisterName}${err?.message ? `\n${err.message}` : ""}`,
181-
);
212+
} finally {
213+
process.removeListener("exit", exitCleanup);
214+
try {
215+
await release?.();
216+
} catch {}
182217
}
183218
}
184219

‎cli/package-lock.json‎

Lines changed: 45 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎cli/package.json‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,7 @@
8686
"prettier-plugin-motoko": "0.13.0",
8787
"promisify-child-process": "4.1.2",
8888
"prompts": "2.4.2",
89+
"proper-lockfile": "4.1.2",
8990
"semver": "7.7.1",
9091
"stream-to-promise": "3.0.0",
9192
"string-width": "7.2.0",
@@ -102,6 +103,7 @@
102103
"@types/ncp": "2.0.8",
103104
"@types/node": "24.0.3",
104105
"@types/prompts": "2.4.9",
106+
"@types/proper-lockfile": "4.1.4",
105107
"@types/semver": "7.5.8",
106108
"@types/stream-to-promise": "2.2.4",
107109
"@types/tar": "6.1.13",

‎cli/tests/build.test.ts‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,23 @@ describe("build", () => {
100100
}
101101
});
102102

103+
test("parallel builds of the same canister both succeed", async () => {
104+
const cwd = path.join(import.meta.dirname, "build/success");
105+
try {
106+
const [a, b] = await Promise.all([
107+
cli(["build", "foo"], { cwd }),
108+
cli(["build", "foo"], { cwd }),
109+
]);
110+
expect(a.exitCode).toBe(0);
111+
expect(b.exitCode).toBe(0);
112+
expect(existsSync(path.join(cwd, ".mops/.build/foo.wasm"))).toBe(true);
113+
expect(existsSync(path.join(cwd, ".mops/.build/foo.did"))).toBe(true);
114+
expect(existsSync(path.join(cwd, ".mops/.build/foo.most"))).toBe(true);
115+
} finally {
116+
cleanFixture(cwd);
117+
}
118+
});
119+
103120
// Regression: bin/mops.js must route through environments/nodejs/cli.js
104121
// so that setWasmBindings() is called before any command runs.
105122
// The dev entry point (npm run mops) uses tsx and always worked;

‎cli/tests/build/success/.gitignore‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,2 +1,3 @@
11
.mops/
22
cli-output-test/
3+
*.most

0 commit comments

Comments
 (0)