Skip to content

Commit

Permalink
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Prefer existing adapter for concurrent Gson.getAdapter calls
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).
Marcono1234 committed Jul 25, 2022

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
1 parent 503c20b commit 6b0b8e7
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
@@ -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;

@@ -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";

/**
@@ -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;
@@ -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;
}
@@ -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;
}
}
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
@@ -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;
@@ -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;

/**
@@ -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);

0 comments on commit 6b0b8e7

Please sign in to comment.