google / google/auto

Nullable field nondeterministically has null check inserted anyhow

Open
#1,338 6 comments 0 reactions 0 assignees View on GitHub
Component: value P3 type=defect
Dominant language
Java
Stars
10.6k
Forks
1.2k
Avg merge
6h 32m
Merged PRs (30d)
13

Description

Summary of issue: A field that is nullable is nondeterministically having a null check in the generated code, depending on what other build tasks occurred.

See https://lists.apache.org/thread/cdw4y50r3hl37z8y7x470m2mddclkqgv (from here you can gather more info from the thread)

Inline, paraphrasing the thread...

To reproduce:

```bash
git clone https://github.com/apache/beam
cd beam
git checkout 4ffeae4d2b800f2df36d2ea2eab549f2204d5691~1
./gradlew :runners:direct-java:compileJava
less
./runners/direct-java/build/generated/sources/annotationProcessor/java/main/org/apache/beam/runners/direct/AutoValue_ImmutableListBundleFactory_CommittedImmutableListBundle.java

git checkout 4ffeae4d2b800f2df36d2ea2eab549f2204d5691
./gradlew :runners:direct-java:compileJava
less
./runners/direct-java/build/generated/sources/annotationProcessor/java/main/org/apache/beam/runners/direct/AutoValue_ImmutableListBundleFactory_CommittedImmutableListBundle.java

./gradlew :runners:direct-java:compileJava --rerun-tasks
less
./runners/direct-java/build/generated/sources/annotationProcessor/java/main/org/apache/beam/runners/direct/AutoValue_ImmutableListBundleFactory_CommittedImmutableListBundle.java
```

The class at https://github.com/apache/beam/blob/1dff59b4ff26310f88f927edfcf44709ed6ea9c2/runners/direct-java/src/main/java/org/apache/beam/runners/direct/ImmutableListBundleFactory.java#L130

```java
abstract static class CommittedImmutableListBundle implements CommittedBundle {
public static CommittedImmutableListBundle create(
@Nullable PCollection pcollection,
StructuralKey key,
Iterable> committedElements,
Instant minElementTimestamp,
Instant synchronizedCompletionTime) {
return new AutoValue_ImmutableListBundleFactory_CommittedImmutableListBundle<>(
pcollection, key, committedElements, minElementTimestamp, synchronizedCompletionTime);
}
...
}
```

extends https://github.com/apache/beam/blob/1dff59b4ff26310f88f927edfcf44709ed6ea9c2/runners/direct-java/src/main/java/org/apache/beam/runners/direct/CommittedBundle.java#L35

```java
interface CommittedBundle extends Bundle> {
/** Returns the PCollection that the elements of this bundle belong to. */
@Override
@Nullable
PCollection getPCollection();

...
}
```

In all cases the `PCollection` field should be nullable.

In a "good" build, the AutoValue generated code constructor does not check for null

```java
AutoValue_ImmutableListBundleFactory_CommittedImmutableListBundle(
@Nullable PCollection PCollection,
StructuralKey key,
Iterable> elements,
Instant minimumTimestamp,
Instant synchronizedProcessingOutputWatermark) {
this.PCollection = PCollection;
```

in a 'bad' build, the autovalue java *is re-generated*, but has an added nullness check:

```java
AutoValue_ImmutableListBundleFactory_CommittedImmutableListBundle(
PCollection PCollection,
StructuralKey key,
Iterable> elements,
Instant minimumTimestamp,
Instant synchronizedProcessingOutputWatermark) {
if (PCollection == null) {
throw new NullPointerException("Null PCollection");
}
this.PCollection = PCollection;
```

and then with `--rerun-tasks` it gets re-re-generated without the nullness check.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.