Skip to content

Commit e837ba2

Browse files
committed
Cache dex syntethic context in dex builder
There is a data race when using workers when there are multiple libraries that include the same class. This is more likely to happen when building multiple android_binary targets in a single invocation. When a cached entry is found, only the dex content is used and the sythetic info is ignored. In those instances, the dex archive will be incorrectly created and might lead to failures during dex merging
1 parent 6982e85 commit e837ba2

File tree

1 file changed

+41
-14
lines changed

1 file changed

+41
-14
lines changed

src/tools/android/java/com/google/devtools/build/android/r8/CompatDexBuilder.java

Lines changed: 41 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ public class CompatDexBuilder {
7373

7474
private static final long ONE_MEG = 1024 * 1024;
7575

76-
private static class ContextConsumer implements SyntheticInfoConsumer {
76+
public static class ContextConsumer implements SyntheticInfoConsumer {
7777

7878
// After compilation this will be non-null iff the compiled class is a D8 synthesized class.
7979
ClassReference sythesizedPrimaryClass = null;
@@ -113,7 +113,7 @@ public void finished() {
113113

114114
private static class DexConsumer implements DexIndexedConsumer {
115115

116-
final ContextConsumer contextConsumer = new ContextConsumer();
116+
ContextConsumer contextConsumer = new ContextConsumer();
117117
byte[] bytes;
118118

119119
@Override
@@ -131,6 +131,10 @@ void setBytes(byte[] byteCode) {
131131
this.bytes = byteCode;
132132
}
133133

134+
void setContextConsumer(ContextConsumer contextConsumer) {
135+
this.contextConsumer = contextConsumer;
136+
}
137+
134138
ContextConsumer getContextConsumer() {
135139
return contextConsumer;
136140
}
@@ -148,17 +152,17 @@ public static void main(String[] args)
148152
PrintStream realStdErr = System.err;
149153

150154
// Set up dexer cache
151-
Cache<DexingKeyR8, byte[]> dexCache =
155+
Cache<DexingKeyR8, DexingEntryR8> dexCache =
152156
CacheBuilder.newBuilder()
153157
// Use at most 200 MB for cache and leave at least 25 MB of heap space alone. For
154158
// reference:
155159
// .class & class.dex files are around 1-5 KB, so this fits ~30K-35K class-dex pairs.
156160
.maximumWeight(min(Runtime.getRuntime().maxMemory() - 25 * ONE_MEG, 200 * ONE_MEG))
157161
.weigher(
158-
new Weigher<DexingKeyR8, byte[]>() {
162+
new Weigher<DexingKeyR8, DexingEntryR8>() {
159163
@Override
160-
public int weigh(DexingKeyR8 key, byte[] value) {
161-
return key.classfileContent().length + value.length;
164+
public int weigh(DexingKeyR8 key, DexingEntryR8 value) {
165+
return key.classfileContent().length + value.dexContent().length;
162166
}
163167
})
164168
.build();
@@ -187,7 +191,7 @@ public int weigh(DexingKeyR8 key, byte[] value) {
187191
}
188192

189193
private int processRequest(
190-
@Nullable Cache<DexingKeyR8, byte[]> dexCache,
194+
@Nullable Cache<DexingKeyR8, DexingEntryR8> dexCache,
191195
DiagnosticsHandler diagnosticsHandler,
192196
List<String> args,
193197
PrintWriter pw) {
@@ -205,7 +209,7 @@ private int processRequest(
205209

206210
@SuppressWarnings("JdkObsolete")
207211
private void dexEntries(
208-
@Nullable Cache<DexingKeyR8, byte[]> dexCache,
212+
@Nullable Cache<DexingKeyR8, DexingEntryR8> dexCache,
209213
List<String> args,
210214
DiagnosticsHandler dexDiagnosticsHandler)
211215
throws IOException, InterruptedException, ExecutionException, OptionsParsingException {
@@ -332,7 +336,7 @@ private void dexEntries(
332336
}
333337

334338
private DexConsumer dexEntry(
335-
@Nullable Cache<DexingKeyR8, byte[]> dexCache,
339+
@Nullable Cache<DexingKeyR8, DexingEntryR8> dexCache,
336340
ZipFile zipFile,
337341
ZipEntry classEntry,
338342
CompilationMode mode,
@@ -349,18 +353,20 @@ private DexConsumer dexEntry(
349353
.setMinApiLevel(minSdkVersion)
350354
.setDisableDesugaring(true)
351355
.setIntermediate(true);
352-
byte[] cachedDexBytes = null;
356+
357+
DexingEntryR8 cachedDexEntry = null;
353358
byte[] classFileBytes = null;
354359
try (InputStream stream = zipFile.getInputStream(classEntry)) {
355360
classFileBytes = ByteStreams.toByteArray(stream);
356361
if (dexCache != null) {
357362
// If the cache exists, check for cache validity.
358-
cachedDexBytes =
363+
cachedDexEntry =
359364
dexCache.getIfPresent(DexingKeyR8.create(mode, minSdkVersion, classFileBytes));
360365
}
361-
if (cachedDexBytes != null) {
366+
if (cachedDexEntry != null) {
362367
// Cache hit: quit early and return the data
363-
consumer.setBytes(cachedDexBytes);
368+
consumer.setBytes(cachedDexEntry.dexContent());
369+
consumer.setContextConsumer(cachedDexEntry.contextConsumer());
364370
return consumer;
365371
}
366372
builder.addClassProgramData(
@@ -371,7 +377,9 @@ private DexConsumer dexEntry(
371377
D8.run(builder.build(), executor);
372378
// After dexing finishes, store the dexed output into the cache.
373379
if (dexCache != null) {
374-
dexCache.put(DexingKeyR8.create(mode, minSdkVersion, classFileBytes), consumer.getBytes());
380+
dexCache.put(
381+
DexingKeyR8.create(mode, minSdkVersion, classFileBytes),
382+
DexingEntryR8.create(consumer.getBytes(), consumer.getContextConsumer()));
375383
}
376384
return consumer;
377385
}
@@ -396,6 +404,25 @@ public static DexingKeyR8 create(
396404
public abstract byte[] classfileContent();
397405
}
398406

407+
/**
408+
* Represents a cache entry in the dex cache.
409+
*/
410+
@AutoValue
411+
public abstract static class DexingEntryR8 {
412+
public static DexingEntryR8 create(
413+
byte[] dexContent, ContextConsumer contextConsumer) {
414+
return new AutoValue_CompatDexBuilder_DexingEntryR8(
415+
dexContent, contextConsumer);
416+
}
417+
418+
@SuppressWarnings("mutable")
419+
public abstract byte[] dexContent();
420+
421+
@SuppressWarnings("mutable")
422+
public abstract ContextConsumer contextConsumer();
423+
}
424+
425+
399426
/**
400427
* Custom implementation of DiagnosticsHandler that writes the info/warning diagnostics messages
401428
* to original System#err stream instead of the WorkerResponse output. This keeps the Bazel

0 commit comments

Comments
 (0)