Unique ServiceLoaderRewriting for each context
Otherwise, methods with service loader calls to the same type in a
wave can race.
Bug: 157430860
Change-Id: Ie7c3123a98ca3c7e6476ef6060a4a099f643ed08
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/ServiceLoaderRewriter.java b/src/main/java/com/android/tools/r8/ir/optimize/ServiceLoaderRewriter.java
index d0ad73a..263cadd 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/ServiceLoaderRewriter.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/ServiceLoaderRewriter.java
@@ -35,8 +35,9 @@
import com.google.common.collect.ImmutableList;
import java.util.ArrayList;
import java.util.Collections;
+import java.util.IdentityHashMap;
import java.util.List;
-import java.util.concurrent.ConcurrentHashMap;
+import java.util.Map;
import java.util.concurrent.atomic.AtomicReference;
/**
@@ -72,8 +73,6 @@
private static final String SERVICE_LOADER_METHOD_PREFIX_NAME = "$load";
private AtomicReference<DexProgramClass> synthesizedClass = new AtomicReference<>();
- private ConcurrentHashMap<DexType, DexEncodedMethod> synthesizedServiceLoaders =
- new ConcurrentHashMap<>();
private final AppView<? extends AppInfoWithLiveness> appView;
@@ -88,6 +87,9 @@
public void rewrite(IRCode code, MethodProcessingId methodProcessingId) {
DexItemFactory factory = appView.dexItemFactory();
InstructionListIterator instructionIterator = code.instructionListIterator();
+ // Create a map from service type to loader methods local to this context since two
+ // service loader calls to the same type in different methods and in the same wave can race.
+ Map<DexType, DexEncodedMethod> synthesizedServiceLoaders = new IdentityHashMap<>();
while (instructionIterator.hasNext()) {
Instruction instruction = instructionIterator.next();
diff --git a/src/test/java/com/android/tools/r8/rewrite/ServiceLoaderMultipleCallsTest.java b/src/test/java/com/android/tools/r8/rewrite/ServiceLoaderMultipleCallsTest.java
new file mode 100644
index 0000000..688a96f
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/rewrite/ServiceLoaderMultipleCallsTest.java
@@ -0,0 +1,141 @@
+// Copyright (c) 2020, the R8 project authors. Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+
+package com.android.tools.r8.rewrite;
+
+import static com.android.tools.r8.utils.codeinspector.Matchers.isPresent;
+import static junit.framework.TestCase.assertEquals;
+import static junit.framework.TestCase.assertNull;
+import static junit.framework.TestCase.assertTrue;
+import static org.hamcrest.MatcherAssert.assertThat;
+
+import com.android.tools.r8.CompilationFailedException;
+import com.android.tools.r8.DataEntryResource;
+import com.android.tools.r8.NeverInline;
+import com.android.tools.r8.TestBase;
+import com.android.tools.r8.TestParameters;
+import com.android.tools.r8.TestParametersCollection;
+import com.android.tools.r8.origin.Origin;
+import com.android.tools.r8.utils.StringUtils;
+import com.android.tools.r8.utils.codeinspector.ClassSubject;
+import com.android.tools.r8.utils.codeinspector.CodeInspector;
+import com.android.tools.r8.utils.codeinspector.InstructionSubject;
+import java.io.IOException;
+import java.nio.file.Path;
+import java.util.ServiceLoader;
+import java.util.concurrent.ExecutionException;
+import java.util.zip.ZipFile;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+
+@RunWith(Parameterized.class)
+public class ServiceLoaderMultipleCallsTest extends TestBase {
+
+ private final TestParameters parameters;
+ private final String EXPECTED_OUTPUT = StringUtils.lines("Hello World!", "Hello World!");
+
+ public interface Service {
+
+ void print();
+ }
+
+ public static class ServiceImpl implements Service {
+
+ @Override
+ public void print() {
+ System.out.println("Hello World!");
+ }
+ }
+
+ public static class ServiceImpl2 implements Service {
+
+ @Override
+ public void print() {
+ System.out.println("Hello World 2!");
+ }
+ }
+
+ public static class MainRunner {
+
+ public static void main(String[] args) {
+ run1();
+ run2();
+ }
+
+ @NeverInline
+ public static void run1() {
+ for (Service x : ServiceLoader.load(Service.class, Service.class.getClassLoader())) {
+ x.print();
+ }
+ }
+
+ @NeverInline
+ public static void run2() {
+ for (Service x : ServiceLoader.load(Service.class, Service.class.getClassLoader())) {
+ x.print();
+ }
+ }
+ }
+
+ @Parameterized.Parameters(name = "{0}")
+ public static TestParametersCollection data() {
+ return getTestParameters().withAllRuntimesAndApiLevels().build();
+ }
+
+ public ServiceLoaderMultipleCallsTest(TestParameters parameters) {
+ this.parameters = parameters;
+ }
+
+ @Test
+ public void testRewritings() throws IOException, CompilationFailedException, ExecutionException {
+ Path path = temp.newFile("out.zip").toPath();
+ testForR8(parameters.getBackend())
+ .addInnerClasses(ServiceLoaderMultipleCallsTest.class)
+ .addKeepMainRule(MainRunner.class)
+ .setMinApi(parameters.getApiLevel())
+ .enableInliningAnnotations()
+ .addDataEntryResources(
+ DataEntryResource.fromBytes(
+ StringUtils.lines(ServiceImpl.class.getTypeName()).getBytes(),
+ "META-INF/services/" + Service.class.getTypeName(),
+ Origin.unknown()))
+ .compile()
+ .writeToZip(path)
+ .run(parameters.getRuntime(), MainRunner.class)
+ .assertSuccessWithOutput(EXPECTED_OUTPUT)
+ .inspect(
+ inspector -> {
+ // Check that we have actually rewritten the calls to ServiceLoader.load.
+ assertEquals(0, getServiceLoaderLoads(inspector, MainRunner.class));
+ // Check that the synthesize service loader class holds two methods, one for each
+ // context.
+ ClassSubject serviceLoaderMethods = inspector.clazz("$$ServiceLoaderMethods");
+ assertThat(serviceLoaderMethods, isPresent());
+ assertEquals(2, serviceLoaderMethods.allMethods().size());
+ });
+
+ // Check that we have removed the service configuration from META-INF/services.
+ ZipFile zip = new ZipFile(path.toFile());
+ assertNull(zip.getEntry("META-INF/services/" + Service.class.getTypeName()));
+ }
+
+ private static long getServiceLoaderLoads(CodeInspector inspector, Class<?> clazz) {
+ ClassSubject classSubject = inspector.clazz(clazz);
+ assertTrue(classSubject.isPresent());
+ return classSubject.allMethods().stream()
+ .mapToLong(
+ method ->
+ method
+ .streamInstructions()
+ .filter(ServiceLoaderMultipleCallsTest::isServiceLoaderLoad)
+ .count())
+ .sum();
+ }
+
+ private static boolean isServiceLoaderLoad(InstructionSubject instruction) {
+ return instruction.isInvokeStatic()
+ && instruction.getMethod().qualifiedName().contains("ServiceLoader.load");
+ }
+}
