Skyframe crash vulnerability with transitions
- Dominant language
- Java
- Stars
- 25.8k
- Forks
- 4.6k
- Avg merge
- 2d 18h
- Merged PRs (30d)
- 75
Description
### Description of the bug:
[StarlarkAttrTransitionProviderTest#.testTransitionOnBuildSetting_noSuchTarget](https://github.com/bazelbuild/bazel/blob/31c4c24667be57195b01548952409d891576c877/src/test/java/com/google/devtools/build/lib/analysis/starlark/StarlarkAttrTransitionProviderTest.java#L2725) is a simple test that transitioning on a non-existent flag returns a "no such target" error.
Comment out these lines:
https://github.com/bazelbuild/bazel/blob/31c4c24667be57195b01548952409d891576c877/src/main/java/com/google/devtools/build/lib/analysis/producers/TransitionApplier.java#L120-L124
then run:
```
$ USE_BAZEL_VERSION=9.0.1rc1 bazelisk test //src/test/java/com/google/devtools/build/lib/analysis/starlark:StarlarkAttrTransitionProviderTest --test_filter=.testTransitionOnBuildSetting_noSuchTarget --test_output=errors
```
It now fails with:
```
java.lang.IllegalStateException: Unexpected analysis error: ConfiguredTargetKey{label=//test/starlark:test, config=BuildConfigurationKey[1d3999f38bc198a7ca1cbe23f490a4a0ab9d8b194fcd3960c0c8e6b62206a781]} -> ErrorInfo{exception=com.google.devtools.build.lib.analysis.starlark.StarlarkTransition$TransitionException: com.google.devtools.build.lib.packages.NoSuchTargetException: no such target '//test/starlark:the-answer': target 'the-answer' not declared in package 'test/starlark' defined by /workspace/test/starlark
```
### Analysis:
The test target `//test/starlark:test` has an explicit exec dependency on `$function_transition_allowlist`.
The test intentionally triggers an error on a *different* Skyframe evaluation: the one that tries to load `//test/starlark:the-answer`, which doesn't exist.
Before the suggested change, https://github.com/bazelbuild/bazel/blob/31c4c24667be57195b01548952409d891576c877/src/main/java/com/google/devtools/build/lib/analysis/producers/TransitionApplier.java#L123
runs and completes before the failing thread runs. Since `applyStarlarkTransition` runs the Starlark thread that evaluates the exec transition, that means the exec transition completes successfully before Skyframe finds errors in other evaluations.
After the suggested change, this now runs: https://github.com/bazelbuild/bazel/blob/31c4c24667be57195b01548952409d891576c877/src/main/java/com/google/devtools/build/lib/analysis/producers/TransitionApplier.java#L129
I'm pretty sure that means Skyframe's currently running `Driver` completes, which gives the failing evaluation time to schedule and fail, then a new `Driver` evaluation runs on `applyStarlarkTransition`, which means the exec transition is evaluated after Skyiframe already knows there's a problem elsewhere.
That means that `StarlarkThread` calls:
https://github.com/bazelbuild/bazel/blob/31c4c24667be57195b01548952409d891576c877/src/main/java/net/starlark/java/eval/StarlarkThread.java#L123-L125
which `DependencyResolver` catches [here](https://github.com/bazelbuild/bazel/blob/31c4c24667be57195b01548952409d891576c877/src/main/java/com/google/devtools/build/lib/skyframe/DependencyResolver.java#L749) as an exceptional case, which triggers the failure.
I experimentally verified that sequencing. i..e I *know* the `StarlarkThread` interrupt triggers only after this change, and before this change the whole evaluation completes before the "no such target" evaluation fails. I'm not quite as confident what the principled answer is, although a simple catching of the `InterruptedException` [here](https://github.com/bazelbuild/bazel/blob/31c4c24667be57195b01548952409d891576c877/src/main/java/com/google/devtools/build/lib/analysis/producers/TransitionApplier.java#L158) should practically resolve it.
### Which category does this issue belong to?
Configurability
### Which operating system are you running Bazel on?
Linux
### What is the output of `bazel info release`?
release 9.0.1rc1
Contributor guide
Research direction
Start with the referenced StarlarkAttrTransitionProviderTest.java case and reproduce it using the provided bazelisk command. Then read the cited TransitionApplier.java flow alongside StarlarkThread.java and DependencyResolver.java to understand the interrupted evaluation sequence. Done means the test reports the intended no-such-target error without an unexpected analysis crash.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- java
- Domain
- build-system
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 42/100