From 4a2c7c55492d2fcae1d8bdbf169d4c4fb710e800 Mon Sep 17 00:00:00 2001
From: Eino-Ville Talvala <etalvala@google.com>
Date: Thu, 9 Jun 2016 13:08:59 -0700
Subject: [PATCH] Camera2: Protect MarshalRegistry against concurrent access

Multiple CameraMetadataNative objects could be reading and writing
to the metadata marshaler registry simultaneously.

This can lead to an infinite loop in the HashMap in the worst case,
so add synchronization against this.

Bug: 29043079
Change-Id: Ic5e9e58a9333b99b4bea87bf790c9fbfadfbbea9
---
 .../camera2/marshal/MarshalRegistry.java      | 82 ++++++++++---------
 1 file changed, 45 insertions(+), 37 deletions(-)

diff --git a/core/java/android/hardware/camera2/marshal/MarshalRegistry.java b/core/java/android/hardware/camera2/marshal/MarshalRegistry.java
index ba821e4770a43..15650879942a5 100644
--- a/core/java/android/hardware/camera2/marshal/MarshalRegistry.java
+++ b/core/java/android/hardware/camera2/marshal/MarshalRegistry.java
@@ -37,7 +37,9 @@ public class MarshalRegistry {
      * @param queryable a non-{@code null} marshal queryable that supports marshaling {@code T}
      */
     public static <T> void registerMarshalQueryable(MarshalQueryable<T> queryable) {
-        sRegisteredMarshalQueryables.add(queryable);
+        synchronized(sMarshalLock) {
+            sRegisteredMarshalQueryables.add(queryable);
+        }
     }
 
     /**
@@ -54,47 +56,50 @@ public class MarshalRegistry {
      */
     @SuppressWarnings("unchecked")
     public static <T> Marshaler<T> getMarshaler(TypeReference<T> typeToken, int nativeType) {
-        // TODO: can avoid making a new token each time by code-genning
-        // the list of type tokens and native types from the keys (at the call sites)
-        MarshalToken<T> marshalToken = new MarshalToken<T>(typeToken, nativeType);
+        synchronized(sMarshalLock) {
+            // TODO: can avoid making a new token each time by code-genning
+            // the list of type tokens and native types from the keys (at the call sites)
+            MarshalToken<T> marshalToken = new MarshalToken<T>(typeToken, nativeType);
 
-        /*
-         * Marshalers are instantiated lazily once they are looked up; successive lookups
-         * will not instantiate new marshalers.
-         */
-        Marshaler<T> marshaler =
-                (Marshaler<T>) sMarshalerMap.get(marshalToken);
-
-        if (sRegisteredMarshalQueryables.size() == 0) {
-            throw new AssertionError("No available query marshalers registered");
-        }
-
-        if (marshaler == null) {
-            // Query each marshaler to see if they support the native/managed type combination
-            for (MarshalQueryable<?> potentialMarshaler : sRegisteredMarshalQueryables) {
-
-                MarshalQueryable<T> castedPotential =
-                        (MarshalQueryable<T>)potentialMarshaler;
-
-                if (castedPotential.isTypeMappingSupported(typeToken, nativeType)) {
-                    marshaler = castedPotential.createMarshaler(typeToken, nativeType);
-                    break;
-                }
-            }
+            /*
+             * Marshalers are instantiated lazily once they are looked up; successive lookups
+             * will not instantiate new marshalers.
+             */
+            Marshaler<T> marshaler =
+                    (Marshaler<T>) sMarshalerMap.get(marshalToken);
 
             if (marshaler == null) {
-                throw new UnsupportedOperationException(
+
+                if (sRegisteredMarshalQueryables.size() == 0) {
+                    throw new AssertionError("No available query marshalers registered");
+                }
+
+                // Query each marshaler to see if they support the native/managed type combination
+                for (MarshalQueryable<?> potentialMarshaler : sRegisteredMarshalQueryables) {
+
+                    MarshalQueryable<T> castedPotential =
+                            (MarshalQueryable<T>)potentialMarshaler;
+
+                    if (castedPotential.isTypeMappingSupported(typeToken, nativeType)) {
+                        marshaler = castedPotential.createMarshaler(typeToken, nativeType);
+                        break;
+                    }
+                }
+
+                if (marshaler == null) {
+                    throw new UnsupportedOperationException(
                         "Could not find marshaler that matches the requested " +
-                                "combination of type reference " +
-                                typeToken + " and native type " +
-                                MarshalHelpers.toStringNativeType(nativeType));
+                        "combination of type reference " +
+                        typeToken + " and native type " +
+                        MarshalHelpers.toStringNativeType(nativeType));
+                }
+
+                // Only put when no cached version exists to avoid +0.5ms lookup per call.
+                sMarshalerMap.put(marshalToken, marshaler);
             }
 
-            // Only put when no cached version exists to avoid +0.5ms lookup per call.
-            sMarshalerMap.put(marshalToken, marshaler);
+            return marshaler;
         }
-
-        return marshaler;
     }
 
     private static class MarshalToken<T> {
@@ -125,9 +130,12 @@ public class MarshalRegistry {
         }
     }
 
-    private static List<MarshalQueryable<?>> sRegisteredMarshalQueryables =
+    // Control access to the static data structures below
+    private static final Object sMarshalLock = new Object();
+
+    private static final List<MarshalQueryable<?>> sRegisteredMarshalQueryables =
             new ArrayList<MarshalQueryable<?>>();
-    private static HashMap<MarshalToken<?>, Marshaler<?>> sMarshalerMap =
+    private static final HashMap<MarshalToken<?>, Marshaler<?>> sMarshalerMap =
             new HashMap<MarshalToken<?>, Marshaler<?>>();
 
     private MarshalRegistry() {