Skip to content

Commit

Permalink
Improve Gson.typeTokenCache usage
Browse files Browse the repository at this point in the history
- Fail fast for null as type (previous null support was broken and would
  have thrown NullPointerException further below anyways)
- Prefer existing adapter in case of concurrency
  • Loading branch information
Marcono1234 committed Jul 25, 2022
1 parent 503c20b commit 8b1e275
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<String>() {
@Override public void write(JsonWriter out, String value) throws IOException {
throw new AssertionError("not needed for test");
}

@Override public String 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 8b1e275

Please sign in to comment.