commit f73fd7894b5f70cff2bfd7f019f14578e333cbcb
author ajurkowski <ajurkowski@google.com> Fri May 21 11:46:16 2021 -0700
committer Copybara-Service <copybara-worker@google.com> Fri May 21 11:47:20 2021 -0700
tree c6c85a8bd7e3007254284d33167638b7f5d5856b
parent 83f623ae7f2eba3e2f655183f5e337c3049154e9

Fix non-deterministic NPE in
`SkyframeActionExecutor::createAndCheckForSymlinks`.

Before we execute an action, we create a directory structure for its outputs.
For each output, we must ensure each ancestor directory exists, up to the
output root. Some of those directories overlap across actions (e.g. `//foo:a`
and `//foo:b` both will output under `foo` directory), which is why we have a
shared cache to avoid stats for those.

The logic resolving ancestor directories first [adds the root to
`knownDirectories`
map](https://github.com/bazelbuild/bazel/blob/e2ae73fe16cbedf095b9e6d5687b06ecf765d794/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeActionExecutor.java#L1368-L1370)
and then, for each directory, [walks it up until it finds
an entry in the
map](https://github.com/bazelbuild/bazel/blob/e2ae73fe16cbedf095b9e6d5687b06ecf765d794/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeActionExecutor.java#L1376).
That is sound for a regular map since every directory is under the root. The
problem is that the [`knownDirectories` map is actually a
cache](https://github.com/bazelbuild/bazel/blob/e2ae73fe16cbedf095b9e6d5687b06ecf765d794/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeActionExecutor.java#L299-L303),
subject to eviction. If that happens for the root, then the loop will
continue until `PathFramgent::getParentDirectory` returns null and NPE.

Fix the check to compare paths to the root as we walk ancestors of paths. This
way checking the cache is just an optimization with no bearing on correctness.

PiperOrigin-RevId: 375131623