From d8a2d52495d457e9cff81b77bbac5c9ccdd7c548 Mon Sep 17 00:00:00 2001 From: mjhalupka Date: Thu, 12 Jul 2018 15:30:25 -0700 Subject: Read a byte array instead of a certain number of bytes that are indicated by a preceding integer when serializing BuildOptions.DiffForReconstruction. Reorder the cache map inserts due to a subtle race condition that can occur. PiperOrigin-RevId: 204376273 --- .../devtools/build/lib/analysis/config/BuildOptions.java | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) (limited to 'src') 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 603d2df6c4..e4be9a3f2c 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 @@ -660,7 +660,6 @@ public final class BuildOptions implements Cloneable, Serializable { context.serialize(diff.differingOptions, bytesOut); context.serialize(diff.extraFirstFragmentClasses, bytesOut); context.serialize(diff.extraSecondFragments, bytesOut); - bytesOut.writeInt32NoTag(diff.baseFingerprint.length); bytesOut.writeByteArrayNoTag(diff.baseFingerprint); context.serialize(diff.checksum, bytesOut); bytesOut.flush(); @@ -693,8 +692,7 @@ public final class BuildOptions implements Cloneable, Serializable { ImmutableSet> extraFirstFragmentClasses = context.deserialize(codedInput); ImmutableList extraSecondFragments = context.deserialize(codedInput); - int fingerprintLength = codedInput.readInt32(); - byte[] baseFingerprint = codedInput.readRawBytes(fingerprintLength); + byte[] baseFingerprint = codedInput.readByteArray(); String checksum = context.deserialize(codedInput); diff = new OptionsDiffForReconstruction( @@ -750,8 +748,12 @@ public final class BuildOptions implements Cloneable, Serializable { @Override public void putBytesFromOptionsDiff(OptionsDiffForReconstruction diff, ByteString bytes) { - diffToByteStringMap.put(diff, bytes); + // We need to insert data into map that will be used for deserialization first in case there + // is a race between two threads. This can occur when one thread has called this method + // and populates one map, giving the other thread a cache hit, but hasn't populated the + // other map yet so on deserialization there is a cache miss. byteStringToDiffMap.put(bytes, diff); + diffToByteStringMap.put(diff, bytes); } @Override -- cgit v1.2.3