diff --git a/src/main/java/com/google/devtools/build/lib/analysis/config/BuildOptions.java b/src/main/java/com/google/devtools/build/lib/analysis/config/BuildOptions.java index cc222218a36bf0..ba866b34e7fdd2 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/config/BuildOptions.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/config/BuildOptions.java @@ -715,8 +715,10 @@ public Class getEncodedClass() { @Override public void serialize( SerializationContext context, BuildOptions options, CodedOutputStream codedOut) - throws IOException { - context.getDependency(OptionsChecksumCache.class).prime(options); + throws SerializationException, IOException { + if (!context.getDependency(OptionsChecksumCache.class).prime(options)) { + throw new SerializationException("Failed to prime cache for " + options.checksum()); + } codedOut.writeStringNoTag(options.checksum()); } @@ -740,15 +742,22 @@ public interface OptionsChecksumCache { /** * Called during deserialization to transform a checksum into a {@link BuildOptions} instance. + * + *

Returns {@code null} when the given checksum is unknown, in which case the codec throws + * {@link SerializationException}. */ + @Nullable BuildOptions getOptions(String checksum); /** * Notifies the cache that it may be necessary to deserialize the given options diff's checksum. * *

Called each time an {@link BuildOptions} instance is serialized. + * + * @return whether this cache was successfully primed, if {@code false} the codec will throw + * {@link SerializationException} */ - void prime(BuildOptions options); + boolean prime(BuildOptions options); } /** @@ -760,13 +769,15 @@ public static final class MapBackedChecksumCache implements OptionsChecksumCache private final ConcurrentMap map = new ConcurrentHashMap<>(); @Override + @Nullable public BuildOptions getOptions(String checksum) { return map.get(checksum); } @Override - public void prime(BuildOptions options) { + public boolean prime(BuildOptions options) { map.putIfAbsent(options.checksum(), options); + return true; } } } diff --git a/src/test/java/com/google/devtools/build/lib/analysis/config/BuildOptionsTest.java b/src/test/java/com/google/devtools/build/lib/analysis/config/BuildOptionsTest.java index e8e70529517ad0..30a697d7660b32 100644 --- a/src/test/java/com/google/devtools/build/lib/analysis/config/BuildOptionsTest.java +++ b/src/test/java/com/google/devtools/build/lib/analysis/config/BuildOptionsTest.java @@ -217,6 +217,29 @@ private static ImmutableList.Builder> makeOptio .add(CppOptions.class); } + @Test + public void serialize_primeFails_throws() throws Exception { + OptionsChecksumCache failToPrimeCache = + new OptionsChecksumCache() { + @Override + public BuildOptions getOptions(String checksum) { + throw new UnsupportedOperationException(); + } + + @Override + public boolean prime(BuildOptions options) { + return false; + } + }; + BuildOptions options = BuildOptions.of(BUILD_CONFIG_OPTIONS); + SerializationContext serializationContext = + new SerializationContext( + ImmutableClassToInstanceMap.of(OptionsChecksumCache.class, failToPrimeCache)); + + assertThrows( + SerializationException.class, () -> TestUtils.toBytes(serializationContext, options)); + } + @Test public void deserialize_unprimedCache_throws() throws Exception { BuildOptions options = BuildOptions.of(BUILD_CONFIG_OPTIONS); @@ -252,7 +275,7 @@ public void deserialize_primedCache_returnsPrimedInstance() throws Exception { // Different checksum cache than the one used for serialization, but it has been primed. OptionsChecksumCache checksumCache = new MapBackedChecksumCache(); - checksumCache.prime(options); + assertThat(checksumCache.prime(options)).isTrue(); DeserializationContext deserializationContext = new DeserializationContext( ImmutableClassToInstanceMap.of(OptionsChecksumCache.class, checksumCache));