Skip to content
Snippets Groups Projects
Unverified Commit 5734d2a5 authored by bazel.build machine account's avatar bazel.build machine account Committed by GitHub
Browse files

[7.2.0] Fix a race condition in IncrementalPackageRoots. (#22127)

Consider the following scenario:

- Top level targets `A` and `B`
- To execute actions in `A`, we need to plant symlinks for
`NestedSet<Package>: [A1, C1, [D1]]`,
- To execute actions in `B`, we need to plant symlinks for
`NestedSet<Package>: [B1, C1, [D1]]`

In the end, we expect to see symlinks to `A1`, `B1`, `C1` and `D1`.

**What went wrong**

With the current code, there are 2 possible race conditions:

1. Transitive NestedSet level:
- Start to plant symlinks for `A`
- `NestedSet [D1]` added to `handledPackageNestedSets`. _No symlink
planted yet_.
- Start to plant symlinks for `B`
- `NestedSet [D1]` seen as "handled" and immediately skipped. Planted
`B1` and `C1`. `B` moves on to execution.
=> actions from `B` that requires `D1` would fail (no such file or
directory).

2. Individual symlink level:
- Start to plant symlinks for `A`
- `C1` added to `lazilyPlantedSymlinks`. _No symlink planted yet_.
- Start to plant symlinks for `B`
- `C1` already seen in `lazilyPlantedSymlinks` and immediately skipped.
Planted `B1` and `D1`. `B` moves on to execution.
=> actions from `B` that requires `C1` would fail (no such file or
directory).

**The Solution**

In order to prevent this race condition, we can plant the symlinks for
top level targets `A` and `B` sequentially. This gives us the guarantee
that: for an action `foo` under a top level target `A`, `foo` is only
executed when all the necessary symlinks for `A` are already planted.

The above scenario would look like:
- Start to plant symlinks for `A`
- The `TopLevelTargetReadyForSymlinkPlanting` event for `B` arrived and
is held in the sequential event queue
- Plant all symlinks. `lazilyPlantedSymlinks: [A1, C1, D1]`. `A` moves
on to execution.
- Start to plant symlinks for `B`
- `NestedSet [D1]` already seen in `handledPackageNestedSets` and
immediately skipped.
- `C1` already seen in `lazilyPlantedSymlinks` and immediately skipped.
- Planted `B1`. `B` moves on to execution.

As an (hopefully not premature) optimization, the symlinks under a
single top level target are planted in parallel.

Fixes #22073

Verified locally with something similar to the repro in
https://github.com/bazelbuild/bazel/issues/22073#issue-2256831560.

PiperOrigin-RevId: 628080361
Change-Id: Ic6c1a6606d26400c46aa98bfeddc844abd075d0a

Commit
https://github.com/bazelbuild/bazel/commit/52adf0b3d14d4af60ece33c95b94be058bafb41b



Co-authored-by: default avatarGoogler <leba@google.com>
parent 6c883e4a
No related merge requests found
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment