Merge remote-tracking branch 'aosp/upstream-master' into master

* aosp/upstream-master:
  Improve diagnostics. Report an error when we fail to register the lambda dump directory. This is achieved by checking InnerClassLambdaMetafactory.dumper's dumpDir.
  Refine assertion by providing more information. When the given path is NOT a directory, output its path.
  Fix Desugar duplicating path for dumped classes.
  Optimize the runtime library for try-with-resources, by reducing the granularity of locks. Now it uses a customized concurrent weak identity hash map.

Test: builds
Change-Id: I8c2231db2ca44066885360aba0296a1d3e6d83f2

4 files changed, 167 insertions, 43 deletions

diff --git a/java/com/google/devtools/build/android/desugar/Desugar.java b/java/com/google/devtools/build/android/desugar/Desugar.java
index b4b4c8e..10c2b99 100644
--- a/java/com/google/devtools/build/android/desugar/Desugar.java
+++ b/java/com/google/devtools/build/android/desugar/Desugar.java
@@ -16,6 +16,7 @@ package com.google.devtools.build.android.desugar;
 import static com.google.common.base.Preconditions.checkArgument;
 import static com.google.common.base.Preconditions.checkNotNull;
 import static com.google.common.base.Preconditions.checkState;
+import static com.google.devtools.build.android.desugar.LambdaClassMaker.LAMBDA_METAFACTORY_DUMPER_PROPERTY;
 import static java.nio.charset.StandardCharsets.ISO_8859_1;
 
 import com.google.auto.value.AutoValue;
@@ -36,6 +37,7 @@ import com.google.errorprone.annotations.MustBeClosed;
 import java.io.IOError;
 import java.io.IOException;
 import java.io.InputStream;
+import java.lang.reflect.Field;
 import java.nio.file.FileVisitResult;
 import java.nio.file.Files;
 import java.nio.file.Path;
@@ -532,6 +534,8 @@ class Desugar {
   public static void main(String[] args) throws Exception {
     // It is important that this method is called first. See its javadoc.
     Path dumpDirectory = createAndRegisterLambdaDumpDirectory();
+    verifyLambdaDumpDirectoryRegistered(dumpDirectory);
+
     Options options = parseCommandLineOptions(args);
     if (options.verbose) {
       System.out.printf("Lambda classes will be written under %s%n", dumpDirectory);
@@ -539,6 +543,29 @@ class Desugar {
     new Desugar(options, dumpDirectory).desugar();
   }
 
+  static void verifyLambdaDumpDirectoryRegistered(Path dumpDirectory) {
+    try {
+      Class<?> klass = Class.forName("java.lang.invoke.InnerClassLambdaMetafactory");
+      Field dumperField = klass.getDeclaredField("dumper");
+      dumperField.setAccessible(true);
+      Object dumperValue = dumperField.get(null);
+      checkNotNull(dumperValue, "Failed to register lambda dump directory '%s'", dumpDirectory);
+
+      Field dumperPathField = dumperValue.getClass().getDeclaredField("dumpDir");
+      dumperPathField.setAccessible(true);
+      Object dumperPath = dumperPathField.get(dumperValue);
+      checkState(
+          dumpDirectory.equals(dumperPath),
+          "Inconsistent lambda dump directories. real='%s', expected='%s'",
+          dumperPath,
+          dumpDirectory);
+    } catch (ReflectiveOperationException e) {
+      // We do not want to crash Desugar, if we cannot load or access these classes or fields.
+      // We aim to provide better diagnostics. If we cannot, just let it go.
+      e.printStackTrace();
+    }
+  }
+
   /**
    * LambdaClassMaker generates lambda classes for us, but it does so by essentially simulating the
    * call to LambdaMetafactory that the JVM would make when encountering an invokedynamic.
@@ -547,15 +574,20 @@ class Desugar {
    * that case the specified directory is used as a temporary dir. Otherwise, it will be set here,
    * before doing anything else since the property is read in the static initializer.
    */
-  private static Path createAndRegisterLambdaDumpDirectory() throws IOException {
-    String propertyValue = System.getProperty(LambdaClassMaker.LAMBDA_METAFACTORY_DUMPER_PROPERTY);
+  static Path createAndRegisterLambdaDumpDirectory() throws IOException {
+    String propertyValue = System.getProperty(LAMBDA_METAFACTORY_DUMPER_PROPERTY);
     if (propertyValue != null) {
-      return Paths.get(propertyValue);
+      Path path = Paths.get(propertyValue).toAbsolutePath();
+      checkState(Files.isDirectory(path), "The path '%s' is not a directory.", path);
+      // It is not necessary to check whether 'path' is an empty directory. It is possible that
+      // LambdaMetafactory is loaded before this class, and there are already lambda classes dumped
+      // into the 'path' folder.
+      // TODO(kmb): Maybe we can empty the folder here.
+      return path;
     }
 
     Path dumpDirectory = Files.createTempDirectory("lambdas");
-    System.setProperty(
-        LambdaClassMaker.LAMBDA_METAFACTORY_DUMPER_PROPERTY, dumpDirectory.toString());
+    System.setProperty(LAMBDA_METAFACTORY_DUMPER_PROPERTY, dumpDirectory.toString());
     deleteTreeOnExit(dumpDirectory);
     return dumpDirectory;
   }
@@ -589,9 +621,8 @@ class Desugar {
   private static ImmutableList<InputOutputPair> toInputOutputPairs(Options options) {
     final ImmutableList.Builder<InputOutputPair> ioPairListbuilder = ImmutableList.builder();
     for (Iterator<Path> inputIt = options.inputJars.iterator(),
-                outputIt = options.outputJars.iterator();
-        inputIt.hasNext();
-        ) {
+            outputIt = options.outputJars.iterator();
+        inputIt.hasNext(); ) {
       ioPairListbuilder.add(InputOutputPair.create(inputIt.next(), outputIt.next()));
     }
     return ioPairListbuilder.build();
diff --git a/java/com/google/devtools/build/android/desugar/LambdaClassMaker.java b/java/com/google/devtools/build/android/desugar/LambdaClassMaker.java
index 155e323..beaeb03 100644
--- a/java/com/google/devtools/build/android/desugar/LambdaClassMaker.java
+++ b/java/com/google/devtools/build/android/desugar/LambdaClassMaker.java
@@ -35,7 +35,8 @@ class LambdaClassMaker {
   private final Map<Path, LambdaInfo> generatedClasses = new LinkedHashMap<>();
 
   public LambdaClassMaker(Path rootDirectory) {
-    checkArgument(Files.isDirectory(rootDirectory));
+    checkArgument(
+        Files.isDirectory(rootDirectory), "The argument '%s' is not a directory.", rootDirectory);
     this.rootDirectory = rootDirectory;
   }
 
diff --git a/java/com/google/devtools/build/android/desugar/TryWithResourcesRewriter.java b/java/com/google/devtools/build/android/desugar/TryWithResourcesRewriter.java
index 38255a1..4093e12 100644
--- a/java/com/google/devtools/build/android/desugar/TryWithResourcesRewriter.java
+++ b/java/com/google/devtools/build/android/desugar/TryWithResourcesRewriter.java
@@ -52,6 +52,8 @@ public class TryWithResourcesRewriter extends ClassVisitor {
       ImmutableSet.of(
           THROWABLE_EXTENSION_INTERNAL_NAME,
           THROWABLE_EXTENSION_INTERNAL_NAME + "$AbstractDesugaringStrategy",
+          THROWABLE_EXTENSION_INTERNAL_NAME + "$ConcurrentWeakIdentityHashMap",
+          THROWABLE_EXTENSION_INTERNAL_NAME + "$ConcurrentWeakIdentityHashMap$WeakKey",
           THROWABLE_EXTENSION_INTERNAL_NAME + "$MimicDesugaringStrategy",
           THROWABLE_EXTENSION_INTERNAL_NAME + "$NullDesugaringStrategy",
           THROWABLE_EXTENSION_INTERNAL_NAME + "$ReuseDesugaringStrategy");
diff --git a/java/com/google/devtools/build/android/desugar/runtime/ThrowableExtension.java b/java/com/google/devtools/build/android/desugar/runtime/ThrowableExtension.java
index 3581fe8..365884b 100644
--- a/java/com/google/devtools/build/android/desugar/runtime/ThrowableExtension.java
+++ b/java/com/google/devtools/build/android/desugar/runtime/ThrowableExtension.java
@@ -15,10 +15,13 @@ package com.google.devtools.build.android.desugar.runtime;
 
 import java.io.PrintStream;
 import java.io.PrintWriter;
+import java.lang.ref.Reference;
+import java.lang.ref.ReferenceQueue;
+import java.lang.ref.WeakReference;
 import java.lang.reflect.Field;
-import java.util.ArrayList;
 import java.util.List;
-import java.util.WeakHashMap;
+import java.util.Vector;
+import java.util.concurrent.ConcurrentHashMap;
 
 /**
  * This is an extension class for java.lang.Throwable. It emulates the methods
@@ -27,7 +30,7 @@ import java.util.WeakHashMap;
  *
  * <p>Note that the Desugar should avoid desugaring this class.
  */
-public class ThrowableExtension {
+public final class ThrowableExtension {
 
   static final AbstractDesugaringStrategy STRATEGY;
   /**
@@ -142,7 +145,7 @@ public class ThrowableExtension {
   }
 
   /** This strategy just delegates all the method calls to java.lang.Throwable. */
-  static class ReuseDesugaringStrategy extends AbstractDesugaringStrategy {
+  static final class ReuseDesugaringStrategy extends AbstractDesugaringStrategy {
 
     @Override
     public void addSuppressed(Throwable receiver, Throwable suppressed) {
@@ -171,17 +174,14 @@ public class ThrowableExtension {
   }
 
   /** This strategy mimics the behavior of suppressed exceptions with a map. */
-  static class MimicDesugaringStrategy extends AbstractDesugaringStrategy {
+  static final class MimicDesugaringStrategy extends AbstractDesugaringStrategy {
 
-    public static final String SUPPRESSED_PREFIX = "Suppressed: ";
-    private final WeakHashMap<Throwable, List<Throwable>> map = new WeakHashMap<>();
+    static final String SUPPRESSED_PREFIX = "Suppressed: ";
+    private final ConcurrentWeakIdentityHashMap map = new ConcurrentWeakIdentityHashMap();
 
     /**
      * Suppress an exception. If the exception to be suppressed is {@receiver} or {@null}, an
      * exception will be thrown.
-     *
-     * @param receiver
-     * @param suppressed
      */
     @Override
     public void addSuppressed(Throwable receiver, Throwable suppressed) {
@@ -191,23 +191,17 @@ public class ThrowableExtension {
       if (suppressed == null) {
         throw new NullPointerException("The suppressed exception cannot be null.");
       }
-      synchronized (this) {
-        List<Throwable> list = map.get(receiver);
-        if (list == null) {
-          list = new ArrayList<>(1);
-          map.put(receiver, list);
-        }
-        list.add(suppressed);
-      }
+      // The returned list is a synchrnozed list.
+      map.get(receiver, /*createOnAbsence=*/true).add(suppressed);
     }
 
     @Override
-    public synchronized Throwable[] getSuppressed(Throwable receiver) {
-      List<Throwable> list = map.get(receiver);
+    public Throwable[] getSuppressed(Throwable receiver) {
+      List<Throwable> list = map.get(receiver, /*createOnAbsence=*/false);
       if (list == null || list.isEmpty()) {
         return EMPTY_THROWABLE_ARRAY;
       }
-      return list.toArray(new Throwable[0]);
+      return list.toArray(EMPTY_THROWABLE_ARRAY);
     }
 
     /**
@@ -218,35 +212,131 @@ public class ThrowableExtension {
      * {@code receiver} and its suppressed exceptions are printed in two different streams.
      */
     @Override
-    public synchronized void printStackTrace(Throwable receiver) {
+    public void printStackTrace(Throwable receiver) {
       receiver.printStackTrace();
-      for (Throwable suppressed : getSuppressed(receiver)) {
-        System.err.print(SUPPRESSED_PREFIX);
-        suppressed.printStackTrace();
+      List<Throwable> suppressedList = map.get(receiver, /*createOnAbsence=*/false);
+      if (suppressedList == null) {
+        return;
+      }
+      synchronized (suppressedList) {
+        for (Throwable suppressed : suppressedList) {
+          System.err.print(SUPPRESSED_PREFIX);
+          suppressed.printStackTrace();
+        }
       }
     }
 
     @Override
-    public synchronized void printStackTrace(Throwable receiver, PrintStream stream) {
+    public void printStackTrace(Throwable receiver, PrintStream stream) {
       receiver.printStackTrace(stream);
-      for (Throwable suppressed : getSuppressed(receiver)) {
-        stream.print(SUPPRESSED_PREFIX);
-        suppressed.printStackTrace(stream);
+      List<Throwable> suppressedList = map.get(receiver, /*createOnAbsence=*/false);
+      if (suppressedList == null) {
+        return;
+      }
+      synchronized (suppressedList) {
+        for (Throwable suppressed : suppressedList) {
+          stream.print(SUPPRESSED_PREFIX);
+          suppressed.printStackTrace(stream);
+        }
       }
     }
 
     @Override
-    public synchronized void printStackTrace(Throwable receiver, PrintWriter writer) {
+    public void printStackTrace(Throwable receiver, PrintWriter writer) {
       receiver.printStackTrace(writer);
-      for (Throwable suppressed : getSuppressed(receiver)) {
-        writer.print(SUPPRESSED_PREFIX);
-        suppressed.printStackTrace(writer);
+      List<Throwable> suppressedList = map.get(receiver, /*createOnAbsence=*/false);
+      if (suppressedList == null) {
+        return;
+      }
+      synchronized (suppressedList) {
+        for (Throwable suppressed : suppressedList) {
+          writer.print(SUPPRESSED_PREFIX);
+          suppressed.printStackTrace(writer);
+        }
+      }
+    }
+  }
+
+  /** A hash map, that is concurrent, weak-key, and identity-hashing. */
+  static final class ConcurrentWeakIdentityHashMap {
+
+    private final ConcurrentHashMap<WeakKey, List<Throwable>> map =
+        new ConcurrentHashMap<>(16, 0.75f, 10);
+    private final ReferenceQueue<Throwable> referenceQueue = new ReferenceQueue<>();
+
+    /**
+     * @param throwable, the key to retrieve or create associated list.
+     * @param createOnAbsence {@code true} to create a new list if there is no value for the key.
+     * @return the associated value with the given {@code throwable}. If {@code createOnAbsence} is
+     *     {@code true}, the returned value will be non-null. Otherwise, it can be {@code null}
+     */
+    public List<Throwable> get(Throwable throwable, boolean createOnAbsence) {
+      deleteEmptyKeys();
+      WeakKey keyForQuery = new WeakKey(throwable, null);
+      List<Throwable> list = map.get(keyForQuery);
+      if (!createOnAbsence) {
+        return list;
+      }
+      if (list != null) {
+        return list;
+      }
+      List<Throwable> newValue = new Vector<>(2);
+      list = map.putIfAbsent(new WeakKey(throwable, referenceQueue), newValue);
+      return list == null ? newValue : list;
+    }
+
+    /** For testing-purpose */
+    int size() {
+      return map.size();
+    }
+
+    void deleteEmptyKeys() {
+      // The ReferenceQueue.poll() is thread-safe.
+      for (Reference<?> key = referenceQueue.poll(); key != null; key = referenceQueue.poll()) {
+        map.remove(key);
+      }
+    }
+
+    private static final class WeakKey extends WeakReference<Throwable> {
+
+      /**
+       * The hash code is used later to retrieve the entry, of which the key is the current weak
+       * key. If the referent is marked for garbage collection and is set to null, we are still able
+       * to locate the entry.
+       */
+      private final int hash;
+
+      public WeakKey(Throwable referent, ReferenceQueue<Throwable> q) {
+        super(referent, q);
+        if (referent == null) {
+          throw new NullPointerException("The referent cannot be null");
+        }
+        hash = System.identityHashCode(referent);
+      }
+
+      @Override
+      public int hashCode() {
+        return hash;
+      }
+
+      @Override
+      public boolean equals(Object obj) {
+        if (obj == null || obj.getClass() != getClass()) {
+          return false;
+        }
+        if (this == obj) {
+          return true;
+        }
+        WeakKey other = (WeakKey) obj;
+        // Note that, after the referent is garbage collected, then the referent will be null.
+        // And the equality test still holds.
+        return this.hash == other.hash && this.get() == other.get();
       }
     }
   }
 
   /** This strategy ignores all suppressed exceptions, which is how retrolambda does. */
-  static class NullDesugaringStrategy extends AbstractDesugaringStrategy {
+  static final class NullDesugaringStrategy extends AbstractDesugaringStrategy {
 
     @Override
     public void addSuppressed(Throwable receiver, Throwable suppressed) {