Propagate `SerializationException` in `NestedSet` expansion methods. PiperOrigin-RevId: 972025054 Change-Id: I551a96f3958b1c67b83b33f0696a2255526d0a5f
diff --git a/src/main/java/com/google/devtools/build/lib/collect/nestedset/NestedSet.java b/src/main/java/com/google/devtools/build/lib/collect/nestedset/NestedSet.java index 76daac3..b58885c 100644 --- a/src/main/java/com/google/devtools/build/lib/collect/nestedset/NestedSet.java +++ b/src/main/java/com/google/devtools/build/lib/collect/nestedset/NestedSet.java
@@ -15,6 +15,7 @@ import static com.google.common.base.Throwables.throwIfInstanceOf; import static com.google.common.base.Throwables.throwIfUnchecked; +import static java.util.concurrent.TimeUnit.NANOSECONDS; import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Preconditions; @@ -31,6 +32,7 @@ import com.google.devtools.build.lib.server.FailureDetails.Interrupted; import com.google.devtools.build.lib.server.FailureDetails.Interrupted.Code; import com.google.devtools.build.lib.skyframe.serialization.FingerprintValueStore.MissingFingerprintValueException; +import com.google.devtools.build.lib.skyframe.serialization.SerializationException; import com.google.devtools.build.lib.skyframe.serialization.VisibleForSerialization; import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec; import com.google.devtools.build.lib.skyframe.serialization.autocodec.SerializationConstant; @@ -49,7 +51,6 @@ import java.util.concurrent.CancellationException; import java.util.concurrent.ExecutionException; import java.util.concurrent.Future; -import java.util.concurrent.TimeUnit; import java.util.concurrent.TimeoutException; import javax.annotation.Nullable; @@ -422,19 +423,20 @@ /** * Returns an immutable list of all unique elements of this set, similar to {@link #toList}, but - * will propagate an {@code InterruptedException} or {@link MissingFingerprintValueException} if - * one is thrown. + * will propagate an {@code InterruptedException}, {@link MissingFingerprintValueException}, or + * {@link SerializationException} if one is thrown. */ public final ImmutableList<E> toListInterruptibly() - throws InterruptedException, MissingFingerprintValueException { + throws InterruptedException, MissingFingerprintValueException, SerializationException { Object actualChildren; - if (children instanceof ListenableFuture) { + if (children instanceof ListenableFuture<?> future) { try { actualChildren = MoreFutures.waitForFutureAndGetWithCheckedException( - (ListenableFuture<Object[]>) children, + future, /* cancelOnInterrupt= */ false, - MissingFingerprintValueException.class); + MissingFingerprintValueException.class, + SerializationException.class); } catch (CancellationException e) { throw new MissingFingerprintValueException(e); } @@ -446,26 +448,33 @@ /** * Returns an immutable list of all unique elements of this set, similar to {@link #toList}, but - * will propagate an {@code InterruptedException} if one is thrown and will throw {@link - * TimeoutException} if this set is deserializing and does not become ready within the given - * timeout. + * supports specifying a timeout for deserialization futures and propagates checked exceptions + * associated with deserialization. * - * <p>Additionally, throws {@link MissingFingerprintValueException} if this nested set {@link - * #isFromStorage} and could not be retrieved. + * <p>The timeout only applies to blocking for the deserialization future to become available. The + * actual list transformation is untimed. * - * <p>Note that the timeout only applies to blocking for the deserialization future to become - * available. The actual list transformation is untimed. + * <p>Checked exceptions are only possible if this nested set {@link #isFromStorage}. + * + * @throws InterruptedException if interrupted while blocking for a deserialization future + * @throws TimeoutException if this set is deserializing and does not become ready within the + * given timeout + * @throws MissingFingerprintValueException if this set could not be retrieved from storage + * @throws SerializationException if one is thrown while deserializing this set */ public final ImmutableList<E> toListWithTimeout(Duration timeout) - throws InterruptedException, TimeoutException, MissingFingerprintValueException { + throws InterruptedException, + TimeoutException, + MissingFingerprintValueException, + SerializationException { Object actualChildren; - if (children instanceof ListenableFuture) { + if (children instanceof ListenableFuture<?> future) { try { - actualChildren = - ((ListenableFuture<Object[]>) children).get(timeout.toNanos(), TimeUnit.NANOSECONDS); + actualChildren = future.get(timeout.toNanos(), NANOSECONDS); } catch (ExecutionException e) { throwIfInstanceOf(e.getCause(), InterruptedException.class); throwIfInstanceOf(e.getCause(), MissingFingerprintValueException.class); + throwIfInstanceOf(e.getCause(), SerializationException.class); throwIfUnchecked(e.getCause()); throw new IllegalStateException(e); } catch (CancellationException e) {
diff --git a/src/main/protobuf/failure_details.proto b/src/main/protobuf/failure_details.proto index a03ee1b..6add643 100644 --- a/src/main/protobuf/failure_details.proto +++ b/src/main/protobuf/failure_details.proto
@@ -1149,6 +1149,7 @@ COVERAGE_NOTES_CREATION_FAILURE = 10 [(metadata) = { exit_code: 1 }]; MODULE_EXPANSION_MISSING_DATA = 11 [(metadata) = { exit_code: 1 }]; MODMAP_INPUT_FILE_READ_FAILURE = 12 [(metadata) = { exit_code: 36 }]; + MODULE_EXPANSION_SERIALIZATION_ERROR = 13 [(metadata) = { exit_code: 1 }]; } Code code = 1;
diff --git a/src/test/java/com/google/devtools/build/lib/collect/nestedset/NestedSetTest.java b/src/test/java/com/google/devtools/build/lib/collect/nestedset/NestedSetTest.java index 50acf34..0abcdd4 100644 --- a/src/test/java/com/google/devtools/build/lib/collect/nestedset/NestedSetTest.java +++ b/src/test/java/com/google/devtools/build/lib/collect/nestedset/NestedSetTest.java
@@ -30,6 +30,7 @@ import com.google.devtools.build.lib.actions.ArtifactRoot; import com.google.devtools.build.lib.actions.util.ActionsTestUtil; import com.google.devtools.build.lib.skyframe.serialization.FingerprintValueStore.MissingFingerprintValueException; +import com.google.devtools.build.lib.skyframe.serialization.SerializationException; import com.google.devtools.build.lib.testutil.TestThread; import com.google.devtools.build.lib.testutil.TestUtils; import com.google.devtools.build.lib.vfs.DigestHashFunction; @@ -451,6 +452,16 @@ } @Test + public void toListInterruptibly_propagatesSerializationException() { + NestedSet<String> deserializingNestedSet = + NestedSet.withFuture( + Order.STABLE_ORDER, + UNKNOWN_DEPTH, + immediateFailedFuture(new SerializationException("test exception"))); + assertThrows(SerializationException.class, deserializingNestedSet::toListInterruptibly); + } + + @Test public void toListInterruptibly_propagatesCancellationAsMissingFingerprintValueException() { NestedSet<String> deserializingNestedSet = NestedSet.withFuture(Order.STABLE_ORDER, UNKNOWN_DEPTH, immediateCancelledFuture()); @@ -482,6 +493,18 @@ } @Test + public void toListWithTimeout_propagatesSerializationException() { + NestedSet<String> deserializingNestedSet = + NestedSet.withFuture( + Order.STABLE_ORDER, + UNKNOWN_DEPTH, + immediateFailedFuture(new SerializationException("test exception"))); + assertThrows( + SerializationException.class, + () -> deserializingNestedSet.toListWithTimeout(Duration.ofNanos(1))); + } + + @Test public void toListWithTimeout_propagatesCancellationAsMissingFingerprintValueException() { NestedSet<String> deserializingNestedSet = NestedSet.withFuture(Order.STABLE_ORDER, UNKNOWN_DEPTH, immediateCancelledFuture());