Skip to content

fix(bazel): ng module compilation workers are subject to linker race-conditions#45393

Closed
devversion wants to merge 1 commit intoangular:masterfrom
devversion:build-disable-linker-ngc-wrapped
Closed

fix(bazel): ng module compilation workers are subject to linker race-conditions#45393
devversion wants to merge 1 commit intoangular:masterfrom
devversion:build-disable-linker-ngc-wrapped

Conversation

@devversion
Copy link
Member

The Bazel NodeJS rules provide two ways of accessing node modules:

  • A linker which creates a node_modules directory in the execroot/or in the runfiles.
  • A patched module resolution where no node modules directory necessarily needs to exist.

The first is the default in rules_nodejs and the second is technically the most idiomatic
resolution mechanism in Bazel (as it matches with a runfile resolution library).

The linker is prone to race conditions in persistent workers, or non-sandbox environments (like
windows). This is because the linker for all workers will operate on a shared execroot directory
and the same node_modules directory is modified all the time / potentially conflicting with other
linker processes from other concurrently-running workers.

We rely on the patched module resolution anyway, but just need to disable the unused linker to avoid
issues like the following:

---8<---8<--- Start of log, file at /private/var/tmp/_bazel_splaktar/280f06d55552a0d01f89f0955b5acd78/bazel-workers/worker-8-TypeScriptCompile.log ---8<---8<---
[link_node_modules.js] An error has been reported: [Error: ENOENT: no such file or directory, unlink 'node_modules'] {
  errno: -2,
  code: 'ENOENT',
  syscall: 'unlink',
  path: 'node_modules'
} Error: ENOENT: no such file or directory, unlink 'node_modules'
---8<---8<--- End of log ---8<---8<---
INFO: Elapsed time: 12.796s, Critical Path: 5.39s
INFO: 645 processes: 477 internal, 12 darwin-sandbox, 156 worker.

@devversion devversion changed the title fix(bazel): ng module compilation works are subject to linker race-conditions fix(bazel): ng module compilation workers are subject to linker race-conditions Mar 19, 2022
…conditions

The Bazel NodeJS rules provide two ways of accessing node modules:

* A linker which creates a `node_modules` directory in the execroot/or in the runfiles.
* A patched module resolution where no node modules directory necessarily needs to exist.

The first is the default in `rules_nodejs` and the second is technically the most idiomatic
resolution mechanism in Bazel (as it matches with a runfile resolution library).

The linker is prone to race conditions in persistent workers, or non-sandbox environments (like
windows). This is because the linker for all workers will operate on a shared `execroot` directory
and the same `node_modules` directory is modified all the time / potentially conflicting with other
linker processes from other concurrently-running workers.

We rely on the patched module resolution anyway, but just need to disable the unused linker to avoid
issues like the following:

```
---8<---8<--- Start of log, file at /private/var/tmp/_bazel_splaktar/280f06d55552a0d01f89f0955b5acd78/bazel-workers/worker-8-TypeScriptCompile.log ---8<---8<---
[link_node_modules.js] An error has been reported: [Error: ENOENT: no such file or directory, unlink 'node_modules'] {
  errno: -2,
  code: 'ENOENT',
  syscall: 'unlink',
  path: 'node_modules'
} Error: ENOENT: no such file or directory, unlink 'node_modules'
---8<---8<--- End of log ---8<---8<---
INFO: Elapsed time: 12.796s, Critical Path: 5.39s
INFO: 645 processes: 477 internal, 12 darwin-sandbox, 156 worker.
```
@devversion devversion force-pushed the build-disable-linker-ngc-wrapped branch from 5f9faff to a214fe2 Compare March 19, 2022 08:05
@devversion devversion marked this pull request as ready for review March 19, 2022 08:59
@devversion devversion added action: review The PR is still awaiting reviews from at least one requested reviewer target: patch This PR is targeted for the next patch release area: bazel Issues related to the published `@angular/bazel` build rules labels Mar 19, 2022
@ngbot ngbot bot modified the milestone: Backlog Mar 19, 2022
@devversion devversion added action: merge The PR is ready for merge by the caretaker and removed action: review The PR is still awaiting reviews from at least one requested reviewer labels Mar 22, 2022
@devversion devversion removed the request for review from josephperrott March 22, 2022 19:31
@dylhunn
Copy link
Contributor

dylhunn commented Mar 24, 2022

This PR was merged into the repository by commit dc72f30.

@dylhunn dylhunn closed this in dc72f30 Mar 24, 2022
dylhunn pushed a commit that referenced this pull request Mar 24, 2022
…conditions (#45393)

The Bazel NodeJS rules provide two ways of accessing node modules:

* A linker which creates a `node_modules` directory in the execroot/or in the runfiles.
* A patched module resolution where no node modules directory necessarily needs to exist.

The first is the default in `rules_nodejs` and the second is technically the most idiomatic
resolution mechanism in Bazel (as it matches with a runfile resolution library).

The linker is prone to race conditions in persistent workers, or non-sandbox environments (like
windows). This is because the linker for all workers will operate on a shared `execroot` directory
and the same `node_modules` directory is modified all the time / potentially conflicting with other
linker processes from other concurrently-running workers.

We rely on the patched module resolution anyway, but just need to disable the unused linker to avoid
issues like the following:

```
---8<---8<--- Start of log, file at /private/var/tmp/_bazel_splaktar/280f06d55552a0d01f89f0955b5acd78/bazel-workers/worker-8-TypeScriptCompile.log ---8<---8<---
[link_node_modules.js] An error has been reported: [Error: ENOENT: no such file or directory, unlink 'node_modules'] {
  errno: -2,
  code: 'ENOENT',
  syscall: 'unlink',
  path: 'node_modules'
} Error: ENOENT: no such file or directory, unlink 'node_modules'
---8<---8<--- End of log ---8<---8<---
INFO: Elapsed time: 12.796s, Critical Path: 5.39s
INFO: 645 processes: 477 internal, 12 darwin-sandbox, 156 worker.
```

PR Close #45393
crapStone pushed a commit to Calciumdibromid/CaBr2 that referenced this pull request Mar 31, 2022
This PR contains the following updates:

| Package | Type | Update | Change |
|---|---|---|---|
| [@angular/animations](https://github.com/angular/angular) | dependencies | patch | [`13.3.0` -> `13.3.1`](https://renovatebot.com/diffs/npm/@angular%2fanimations/13.3.0/13.3.1) |
| [@angular/common](https://github.com/angular/angular) | dependencies | patch | [`13.3.0` -> `13.3.1`](https://renovatebot.com/diffs/npm/@angular%2fcommon/13.3.0/13.3.1) |
| [@angular/compiler](https://github.com/angular/angular) | dependencies | patch | [`13.3.0` -> `13.3.1`](https://renovatebot.com/diffs/npm/@angular%2fcompiler/13.3.0/13.3.1) |
| [@angular/compiler-cli](https://github.com/angular/angular) | devDependencies | patch | [`13.3.0` -> `13.3.1`](https://renovatebot.com/diffs/npm/@angular%2fcompiler-cli/13.3.0/13.3.1) |
| [@angular/core](https://github.com/angular/angular) | dependencies | patch | [`13.3.0` -> `13.3.1`](https://renovatebot.com/diffs/npm/@angular%2fcore/13.3.0/13.3.1) |
| [@angular/forms](https://github.com/angular/angular) | dependencies | patch | [`13.3.0` -> `13.3.1`](https://renovatebot.com/diffs/npm/@angular%2fforms/13.3.0/13.3.1) |
| [@angular/platform-browser](https://github.com/angular/angular) | dependencies | patch | [`13.3.0` -> `13.3.1`](https://renovatebot.com/diffs/npm/@angular%2fplatform-browser/13.3.0/13.3.1) |
| [@angular/platform-browser-dynamic](https://github.com/angular/angular) | dependencies | patch | [`13.3.0` -> `13.3.1`](https://renovatebot.com/diffs/npm/@angular%2fplatform-browser-dynamic/13.3.0/13.3.1) |

---

### Release Notes

<details>
<summary>angular/angular</summary>

### [`v13.3.1`](https://github.com/angular/angular/blob/HEAD/CHANGELOG.md#&#8203;1331-2022-03-30)

[Compare Source](angular/angular@13.3.0...13.3.1)

##### bazel

| Commit | Type | Description |
| -- | -- | -- |
| [960e42b2ac](angular/angular@960e42b) | fix | ng module compilation workers are subject to linker race-conditions ([#&#8203;45393](angular/angular#45393)) |

##### compiler

| Commit | Type | Description |
| -- | -- | -- |
| [3714305f84](angular/angular@3714305) | fix | scope css rules within `@layer` blocks ([#&#8203;45396](angular/angular#45396)) |

##### compiler-cli

| Commit | Type | Description |
| -- | -- | -- |
| [7f53c0f4ac](angular/angular@7f53c0f) | fix | handle inline type-check blocks in nullish coalescing extended check ([#&#8203;45478](angular/angular#45478)) |

#### Special Thanks

AlirezaEbrahimkhani, Andrew Kushnir, Andrew Scott, Ben Brook, Dylan Hunn, George Kalpakas, JiaLiPassion, Joey Perrott, JoostK, Mike, Paul Gschwendtner, Willian Corrêa, arturovt, dario-piotrowicz, khai and mgechev

<!-- CHANGELOG SPLIT MARKER -->

</details>

---

### Configuration

📅 **Schedule**: At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about these updates again.

---

 - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, click this checkbox.

---

This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).

Co-authored-by: cabr2-bot <cabr2.help@gmail.com>
Reviewed-on: https://codeberg.org/Calciumdibromid/CaBr2/pulls/1277
Reviewed-by: 6543 <6543@noreply.codeberg.org>
Co-authored-by: Calciumdibromid Bot <cabr2_bot@noreply.codeberg.org>
Co-committed-by: Calciumdibromid Bot <cabr2_bot@noreply.codeberg.org>
@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Apr 24, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

action: merge The PR is ready for merge by the caretaker area: bazel Issues related to the published `@angular/bazel` build rules target: patch This PR is targeted for the next patch release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants