Skip to content

Commit

Permalink
Prefer existing adapter for concurrent Gson.getAdapter calls (#2153)
Browse files Browse the repository at this point in the history
Additionally fail fast for null as type (previous null support was broken
and would have thrown NullPointerException further below anyways).
  • Loading branch information
Marcono1234 authored Aug 1, 2022
1 parent a45c557 commit 4552db2
Show file tree
Hide file tree
Showing 2 changed files with 75 additions and 4 deletions.
16 changes: 12 additions & 4 deletions gson/src/main/java/com/google/gson/Gson.java
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@
import java.util.List;
import java.util.Map;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.ConcurrentMap;
import java.util.concurrent.atomic.AtomicLong;
import java.util.concurrent.atomic.AtomicLongArray;

Expand Down Expand Up @@ -143,7 +144,6 @@ public final class Gson {
static final ToNumberStrategy DEFAULT_OBJECT_TO_NUMBER_STRATEGY = ToNumberPolicy.DOUBLE;
static final ToNumberStrategy DEFAULT_NUMBER_TO_NUMBER_STRATEGY = ToNumberPolicy.LAZILY_PARSED_NUMBER;

private static final TypeToken<?> NULL_KEY_SURROGATE = TypeToken.get(Object.class);
private static final String JSON_NON_EXECUTABLE_PREFIX = ")]}'\n";

/**
Expand All @@ -156,7 +156,7 @@ public final class Gson {
private final ThreadLocal<Map<TypeToken<?>, FutureTypeAdapter<?>>> calls
= new ThreadLocal<>();

private final Map<TypeToken<?>, TypeAdapter<?>> typeTokenCache = new ConcurrentHashMap<>();
private final ConcurrentMap<TypeToken<?>, TypeAdapter<?>> typeTokenCache = new ConcurrentHashMap<>();

private final ConstructorConstructor constructorConstructor;
private final JsonAdapterAnnotationTypeAdapterFactory jsonAdapterFactory;
Expand Down Expand Up @@ -504,7 +504,10 @@ private static TypeAdapter<AtomicLongArray> atomicLongArrayAdapter(final TypeAda
*/
@SuppressWarnings("unchecked")
public <T> TypeAdapter<T> getAdapter(TypeToken<T> type) {
TypeAdapter<?> cached = typeTokenCache.get(type == null ? NULL_KEY_SURROGATE : type);
if (type == null) {
throw new NullPointerException("type must not be null");
}
TypeAdapter<?> cached = typeTokenCache.get(type);
if (cached != null) {
return (TypeAdapter<T>) cached;
}
Expand All @@ -530,8 +533,13 @@ public <T> TypeAdapter<T> getAdapter(TypeToken<T> type) {
for (TypeAdapterFactory factory : factories) {
TypeAdapter<T> candidate = factory.create(this, type);
if (candidate != null) {
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);
typeTokenCache.put(type, candidate);
return candidate;
}
}
Expand Down
63 changes: 63 additions & 0 deletions gson/src/test/java/com/google/gson/GsonTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
package com.google.gson;

import com.google.gson.internal.Excluder;
import com.google.gson.reflect.TypeToken;
import com.google.gson.stream.JsonReader;
import com.google.gson.stream.JsonWriter;
import com.google.gson.stream.MalformedJsonException;
Expand All @@ -29,6 +30,7 @@
import java.util.ArrayList;
import java.util.Collections;
import java.util.HashMap;
import java.util.concurrent.atomic.AtomicReference;
import junit.framework.TestCase;

/**
Expand Down Expand Up @@ -89,6 +91,67 @@ private static final class TestTypeAdapter extends TypeAdapter<Object> {
@Override public Object read(JsonReader in) throws IOException { return null; }
}

public void testGetAdapter_Null() {
Gson gson = new Gson();
try {
gson.getAdapter((TypeToken<?>) null);
fail();
} catch (NullPointerException e) {
assertEquals("type must not be null", e.getMessage());
}
}

public void testGetAdapter_Concurrency() {
final AtomicReference<TypeAdapter<?>> threadAdapter = new AtomicReference<>();
final Class<?> requestedType = Number.class;

Gson gson = new GsonBuilder()
.registerTypeAdapterFactory(new TypeAdapterFactory() {
private volatile boolean isFirstCall = true;

@Override public <T> TypeAdapter<T> create(final Gson gson, TypeToken<T> type) {
if (isFirstCall) {
isFirstCall = false;

// Create a separate thread which requests an adapter for the same type
// This will cause this factory to return a different adapter instance than
// the one it is currently creating
Thread thread = new Thread() {
@Override public void run() {
threadAdapter.set(gson.getAdapter(requestedType));
}
};
thread.start();
try {
thread.join();
} catch (InterruptedException e) {
throw new RuntimeException(e);
}
}

// Create a new dummy adapter instance

@SuppressWarnings("unchecked")
TypeAdapter<T> r = (TypeAdapter<T>) new TypeAdapter<Number>() {
@Override public void write(JsonWriter out, Number value) throws IOException {
throw new AssertionError("not needed for test");
}

@Override public Number read(JsonReader in) throws IOException {
throw new AssertionError("not needed for test");
}
};
return r;
}
})
.create();

TypeAdapter<?> adapter = gson.getAdapter(requestedType);
assertNotNull(adapter);
// Should be the same adapter instance the concurrent thread received
assertSame(threadAdapter.get(), adapter);
}

public void testNewJsonWriter_Default() throws IOException {
StringWriter writer = new StringWriter();
JsonWriter jsonWriter = new Gson().newJsonWriter(writer);
Expand Down

0 comments on commit 4552db2

Please sign in to comment.