Skip to content

feat(cli): add per-canister build lock to prevent parallel clobbering#472

Merged
Kamirus merged 3 commits intomainfrom
feat/per-canister-build-lock
Apr 1, 2026
Merged

feat(cli): add per-canister build lock to prevent parallel clobbering#472
Kamirus merged 3 commits intomainfrom
feat/per-canister-build-lock

Conversation

@Kamirus
Copy link
Copy Markdown
Collaborator

@Kamirus Kamirus commented Apr 1, 2026

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

  • New test: two parallel mops build foo via Promise.all — both succeed
  • Existing tests pass (error paths, custom output dir, CLI flags, managed flag warnings)
  • Pre-commit hooks pass (prettier, eslint, tsc)

Kamirus added 3 commits April 1, 2026 09:34
Parallel `mops build` invocations targeting the same canister race on
writing .wasm, .did, and .most files, producing corrupted output.

Add per-canister file locking via `proper-lockfile` so that concurrent
builds of the same canister are serialized while different canisters
can still build in parallel.

Made-with: Cursor
- Wrap release() in try-catch in finally to prevent masking build errors
- Reduce retry count from 120 to 60 to align with 5-minute stale timeout
- Update ELOCKED error message to describe timeout, not a live contention
- Document intentional redundancy of exitCleanup with signal-exit
- Rename test to match what it actually asserts

Made-with: Cursor
@Kamirus Kamirus marked this pull request as ready for review April 1, 2026 09:57
@Kamirus Kamirus requested a review from a team as a code owner April 1, 2026 09:57
}

// proper-lockfile registers its own signal-exit handler, but it doesn't reliably
// fire on process.exit(). This manual handler covers that gap. Double-unlock is
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could get rid of this exitCleanup after refactoring the cliError to throw an exception and not process.exit, but it is a huge refactor so I don't know if we want to commit to it : #473

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, we can evaluate it separately after merging this PR. Exit cleanup seems like a reasonable / acceptable way to do this imo.

@Kamirus Kamirus requested a review from rvanasa April 1, 2026 09:59
}

// proper-lockfile registers its own signal-exit handler, but it doesn't reliably
// fire on process.exit(). This manual handler covers that gap. Double-unlock is
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, we can evaluate it separately after merging this PR. Exit cleanup seems like a reasonable / acceptable way to do this imo.

@Kamirus Kamirus merged commit 21ee4c4 into main Apr 1, 2026
29 checks passed
@Kamirus Kamirus deleted the feat/per-canister-build-lock branch April 1, 2026 19:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants