Skip to content

Commit 1cf392f

Browse files
justinhorvitzcopybara-github
authored andcommitted
Allow OptionsChecksumCache to fail to prime itself, resulting in a SerializationException.
PiperOrigin-RevId: 565814705 Change-Id: Id4f43c31f2f51c8a871bbbbdfffc6e570404d710
1 parent d8d3593 commit 1cf392f

File tree

2 files changed

+39
-5
lines changed

2 files changed

+39
-5
lines changed

src/main/java/com/google/devtools/build/lib/analysis/config/BuildOptions.java

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -715,8 +715,10 @@ public Class<BuildOptions> getEncodedClass() {
715715
@Override
716716
public void serialize(
717717
SerializationContext context, BuildOptions options, CodedOutputStream codedOut)
718-
throws IOException {
719-
context.getDependency(OptionsChecksumCache.class).prime(options);
718+
throws SerializationException, IOException {
719+
if (!context.getDependency(OptionsChecksumCache.class).prime(options)) {
720+
throw new SerializationException("Failed to prime cache for " + options.checksum());
721+
}
720722
codedOut.writeStringNoTag(options.checksum());
721723
}
722724

@@ -740,15 +742,22 @@ public interface OptionsChecksumCache {
740742

741743
/**
742744
* Called during deserialization to transform a checksum into a {@link BuildOptions} instance.
745+
*
746+
* <p>Returns {@code null} when the given checksum is unknown, in which case the codec throws
747+
* {@link SerializationException}.
743748
*/
749+
@Nullable
744750
BuildOptions getOptions(String checksum);
745751

746752
/**
747753
* Notifies the cache that it may be necessary to deserialize the given options diff's checksum.
748754
*
749755
* <p>Called each time an {@link BuildOptions} instance is serialized.
756+
*
757+
* @return whether this cache was successfully primed, if {@code false} the codec will throw
758+
* {@link SerializationException}
750759
*/
751-
void prime(BuildOptions options);
760+
boolean prime(BuildOptions options);
752761
}
753762

754763
/**
@@ -760,13 +769,15 @@ public static final class MapBackedChecksumCache implements OptionsChecksumCache
760769
private final ConcurrentMap<String, BuildOptions> map = new ConcurrentHashMap<>();
761770

762771
@Override
772+
@Nullable
763773
public BuildOptions getOptions(String checksum) {
764774
return map.get(checksum);
765775
}
766776

767777
@Override
768-
public void prime(BuildOptions options) {
778+
public boolean prime(BuildOptions options) {
769779
map.putIfAbsent(options.checksum(), options);
780+
return true;
770781
}
771782
}
772783
}

src/test/java/com/google/devtools/build/lib/analysis/config/BuildOptionsTest.java

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -217,6 +217,29 @@ private static ImmutableList.Builder<Class<? extends FragmentOptions>> makeOptio
217217
.add(CppOptions.class);
218218
}
219219

220+
@Test
221+
public void serialize_primeFails_throws() throws Exception {
222+
OptionsChecksumCache failToPrimeCache =
223+
new OptionsChecksumCache() {
224+
@Override
225+
public BuildOptions getOptions(String checksum) {
226+
throw new UnsupportedOperationException();
227+
}
228+
229+
@Override
230+
public boolean prime(BuildOptions options) {
231+
return false;
232+
}
233+
};
234+
BuildOptions options = BuildOptions.of(BUILD_CONFIG_OPTIONS);
235+
SerializationContext serializationContext =
236+
new SerializationContext(
237+
ImmutableClassToInstanceMap.of(OptionsChecksumCache.class, failToPrimeCache));
238+
239+
assertThrows(
240+
SerializationException.class, () -> TestUtils.toBytes(serializationContext, options));
241+
}
242+
220243
@Test
221244
public void deserialize_unprimedCache_throws() throws Exception {
222245
BuildOptions options = BuildOptions.of(BUILD_CONFIG_OPTIONS);
@@ -252,7 +275,7 @@ public void deserialize_primedCache_returnsPrimedInstance() throws Exception {
252275

253276
// Different checksum cache than the one used for serialization, but it has been primed.
254277
OptionsChecksumCache checksumCache = new MapBackedChecksumCache();
255-
checksumCache.prime(options);
278+
assertThat(checksumCache.prime(options)).isTrue();
256279
DeserializationContext deserializationContext =
257280
new DeserializationContext(
258281
ImmutableClassToInstanceMap.of(OptionsChecksumCache.class, checksumCache));

0 commit comments

Comments
 (0)