Fix non-threadsafe creation of adapter for type with cyclic dependency

gson/src/main/java/com/google/gson/Gson.java

@@ -52,7 +52,8 @@
 import java.text.DateFormat;
 import java.util.ArrayList;
 import java.util.Collections;
-import java.util.HashMap;
+import java.util.Iterator;
+import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.Objects;
@@ -161,8 +162,8 @@ public final class Gson {
    * lookup would stack overflow. We cheat by returning a proxy type adapter.
    * The proxy is wired up once the initial adapter has been created.
    */
-  private final ThreadLocal<Map<TypeToken<?>, FutureTypeAdapter<?>>> calls
-      = new ThreadLocal<>();
+  // Uses LinkedHashMap because iteration order is important, see getAdapter() implementation below
+  private final ThreadLocal<LinkedHashMap<TypeToken<?>, TypeAdapter<?>>> calls = new ThreadLocal<>();
 
   private final ConcurrentMap<TypeToken<?>, TypeAdapter<?>> typeTokenCache = new ConcurrentHashMap<>();
 
@@ -523,46 +524,76 @@ public <T> TypeAdapter<T> getAdapter(TypeToken<T> type) {
       return adapter;
     }
 
-    Map<TypeToken<?>, FutureTypeAdapter<?>> threadCalls = calls.get();
-    boolean requiresThreadLocalCleanup = false;
+    LinkedHashMap<TypeToken<?>, TypeAdapter<?>> threadCalls = calls.get();
+    boolean isInitialAdapterRequest = false;
     if (threadCalls == null) {
-      threadCalls = new HashMap<>();
+      threadCalls = new LinkedHashMap<>();
       calls.set(threadCalls);
-      requiresThreadLocalCleanup = true;
+      isInitialAdapterRequest = true;
     }
 
     // the key and value type parameters always agree
     @SuppressWarnings("unchecked")
-    FutureTypeAdapter<T> ongoingCall = (FutureTypeAdapter<T>) threadCalls.get(type);
+    TypeAdapter<T> ongoingCall = (TypeAdapter<T>) threadCalls.get(type);
     if (ongoingCall != null) {
       return ongoingCall;
     }
 
+    int existingAdaptersCount = threadCalls.size();
+    boolean foundCandidate = false;
     try {
       FutureTypeAdapter<T> call = new FutureTypeAdapter<>();
       threadCalls.put(type, call);
 
       for (TypeAdapterFactory factory : factories) {
         TypeAdapter<T> candidate = factory.create(this, type);
         if (candidate != null) {
-          @SuppressWarnings("unchecked")
-          TypeAdapter<T> existingAdapter = (TypeAdapter<T>) typeTokenCache.putIfAbsent(type, candidate);
-          // If other thread concurrently added adapter prefer that one instead
-          if (existingAdapter != null) {
-            candidate = existingAdapter;
-          }
-
           call.setDelegate(candidate);
+          // Replace future adapter with actual adapter
+          threadCalls.put(type, candidate);
+
+          if (isInitialAdapterRequest) {
+            // Publish resolved adapters to all threads
+            // Can only do this for the initial request because cyclic dependency TypeA -> TypeB -> TypeA
+            // would otherwise publish adapter for TypeB which uses not yet resolved adapter for TypeA
+            // See https://github.com/google/gson/issues/625
+            for (Map.Entry<TypeToken<?>, TypeAdapter<?>> resolvedAdapterEntry : threadCalls.entrySet()) {
+              typeTokenCache.putIfAbsent(resolvedAdapterEntry.getKey(), resolvedAdapterEntry.getValue());
+            }
+
+            @SuppressWarnings("unchecked")
+            TypeAdapter<T> actualAdapter = (TypeAdapter<T>) typeTokenCache.get(type);
+            // Prefer the actual adapter, in case putIfAbsent call above had no effect because other
+            // thread already concurrently added other adapter instance for the same type
+            candidate = actualAdapter;
+          }
+          foundCandidate = true;
           return candidate;
         }
       }
       throw new IllegalArgumentException("GSON (" + GsonBuildConfig.VERSION + ") cannot handle " + type);
     } finally {
-      threadCalls.remove(type);
-
-      if (requiresThreadLocalCleanup) {
+      if (isInitialAdapterRequest) {
         calls.remove();
       }
+      if (!foundCandidate) {
+        Iterator<TypeAdapter<?>> adaptersIterator = threadCalls.values().iterator();
+        // Skip existing non-broken adapters
+        for (; existingAdaptersCount > 0; existingAdaptersCount--) {
+          adaptersIterator.next();
+        }
+        // Remove this future adapter and all nested ones because they might
+        // refer to broken adapters
+        while (adaptersIterator.hasNext()) {
+          TypeAdapter<?> brokenAdapter = adaptersIterator.next();
+          if (brokenAdapter instanceof FutureTypeAdapter) {
+            // Mark adapter as broken so user sees useful exception message in
+            // case TypeAdapterFactory leaks reference to broken adapter
+            ((FutureTypeAdapter<?>) brokenAdapter).markBroken();
+          }
+          adaptersIterator.remove();
+        }
+      }
     }
   }
 
@@ -1320,7 +1351,8 @@ public <T> T fromJson(JsonElement json, TypeToken<T> typeOfT) throws JsonSyntaxE
   }
 
   static class FutureTypeAdapter<T> extends SerializationDelegatingTypeAdapter<T> {
-    private TypeAdapter<T> delegate;
+    private TypeAdapter<T> delegate = null;
+    private boolean isBroken = false;
 
     public void setDelegate(TypeAdapter<T> typeAdapter) {
       if (delegate != null) {
@@ -1329,9 +1361,20 @@ public void setDelegate(TypeAdapter<T> typeAdapter) {
       delegate = typeAdapter;
     }
 
+    public void markBroken() {
+      isBroken = true;
+    }
+
     private TypeAdapter<T> delegate() {
+      TypeAdapter<T> delegate = this.delegate;
+      if (isBroken) {
+        throw new IllegalStateException("Broken adapter has been leaked by TypeAdapterFactory");
+      }
       if (delegate == null) {
-        throw new IllegalStateException("Delegate has not been set yet");
+        // Can occur when adapter is leaked to other thread or when adapter is used for (de-)serialization
+        // directly within the TypeAdapterFactory which requested it
+        throw new IllegalStateException("Adapter for type with cyclic dependency has been used"
+            + " before dependency has been resolved");
       }
       return delegate;
     }