Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rare exception during POJO serialization #764

Closed
huguesb opened this issue Jan 2, 2016 · 6 comments
Closed

Rare exception during POJO serialization #764

huguesb opened this issue Jan 2, 2016 · 6 comments
Labels

Comments

@huguesb
Copy link

huguesb commented Jan 2, 2016

We're seeing what appears to be some sort of concurrency-related bug in our CI system. It occurs very rarely and only when multiple threads are serializing similar objects.

java.lang.IllegalStateException: null
    at com.google.gson.Gson$FutureTypeAdapter.write(Gson.java:890) ~[gson-2.2.4.jar:na]
    at com.google.gson.internal.bind.TypeAdapterRuntimeTypeWrapper.write(TypeAdapterRuntimeTypeWrapper.java:68) ~[gson-2.2.4.jar:na]
    at com.google.gson.internal.bind.ReflectiveTypeAdapterFactory$1.write(ReflectiveTypeAdapterFactory.java:89) ~[gson-2.2.4.jar:na]
    at com.google.gson.internal.bind.ReflectiveTypeAdapterFactory$Adapter.write(ReflectiveTypeAdapterFactory.java:195) ~[gson-2.2.4.jar:na]
    at com.google.gson.internal.bind.TypeAdapterRuntimeTypeWrapper.write(TypeAdapterRuntimeTypeWrapper.java:68) ~[gson-2.2.4.jar:na]
    at com.google.gson.internal.bind.CollectionTypeAdapterFactory$Adapter.write(CollectionTypeAdapterFactory.java:96) ~[gson-2.2.4.jar:na]
    at com.google.gson.internal.bind.CollectionTypeAdapterFactory$Adapter.write(CollectionTypeAdapterFactory.java:60) ~[gson-2.2.4.jar:na]
    at com.google.gson.internal.bind.TypeAdapterRuntimeTypeWrapper.write(TypeAdapterRuntimeTypeWrapper.java:68) ~[gson-2.2.4.jar:na]
    at com.google.gson.internal.bind.ReflectiveTypeAdapterFactory$1.write(ReflectiveTypeAdapterFactory.java:89) ~[gson-2.2.4.jar:na]
    at com.google.gson.internal.bind.ReflectiveTypeAdapterFactory$Adapter.write(ReflectiveTypeAdapterFactory.java:195) ~[gson-2.2.4.jar:na]
    at com.google.gson.Gson.toJson(Gson.java:593) ~[gson-2.2.4.jar:na]

The hierarchy being serialized is as follows:

public class ChildrenList {
    public final @Nullable String parent;
    public final List<Folder> folders;
    public final List<File> files;
}

public class CommonMetadata {
    public final String id;
    public final String name;
    public final @Nullable String parent;
    public final @Nullable ParentPath path;
}

public class Folder extends CommonMetadata
{
    public final boolean is_shared;
    public final @Nullable String sid;
    public final @Nullable ChildrenList children;
}

A ChildrenList object is being serialized and dumping the content of the output buffer shows that the error happens in a field of the first Folder element of the folders field. To be precise, the partial output looks like:

{"parent":"8c32f02fed08392bccf015ee0b6274d300000000000000000000000000000000","folders":[{"is_shared":false

A single static Gson instance is shared by multiple threads serializing POJO into HTTP response bodies and is initialized as follows:

    private static final Gson _gson = new GsonBuilder()
            .registerTypeAdapter(Date.class, new DateTypeAdapter())
            .setFieldNamingPolicy(FieldNamingPolicy.LOWER_CASE_WITH_UNDERSCORES)
            .create();

    /**
     * Enforce ISO 8601 format and UTC timezone for date serialization
     */
    public static class DateTypeAdapter implements JsonSerializer<Date>
    {
        // DateFormat is not thread-safe
        private final ThreadLocal<DateFormat> dateFormat = new ThreadLocal<>();

        @Override
        public JsonElement serialize(Date date, Type type,
                JsonSerializationContext jsonSerializationContext) {
            DateFormat fmt = dateFormat.get();
            if (fmt == null) {
                fmt = new SimpleDateFormat("yyyy-MM-dd'T'HH:mm:ss'Z'");
                fmt.setTimeZone(TimeZone.getTimeZone("UTC"));
                dateFormat.set(fmt);
            }
            String dateFormatAsString = fmt.format(date);
            return new JsonPrimitive(dateFormatAsString);
        }
    }

Has anyone ever run into a similar issue? Is there anything we can do to get a more helpful stack trace when the error happens?

@pietern
Copy link

pietern commented Jan 6, 2016

I'm seeing the same thing. Happens if two or more threads try to serialize the same recursive type for the first time at the same time. Verified this is also an issue with GSON 2.5.

@pietern
Copy link

pietern commented Jan 7, 2016

Here's what I think is happening:

  • Thread A creates adapter for Folder, storing a FutureAdapter for it in a thread local
  • Thread B creates adapter for Folder, storing a FutureAdapter for it in a thread local
  • Thread A recursively creates adapter for ChildrenList
  • Thread A recursively creates adapter for Folder, which resolves to the FutureAdapter
  • Thread A caches the newly created adapter for ChildrenList (with FutureAdapter for Folder)
  • Thread B recursively creates adapter for ChildrenList and uses this cached adapter
  • Thread B never sets the delegate for the FutureAdapter of Folder in the ChildrenList adapter
  • Thread B tries to use the Folder adapter, before thread A sets the delegate on the FutureAdapter
  • IllegalStateException

To fix this, the implementation of getAdapter must not externalize/cache newly created adapters IF there is one or more FutureAdapters available in its thread's thread local. These adapters should be cached in a thread local and only be cached in GSON's global cache after the last delegate has been set.

@swankjesse
Copy link
Collaborator

Yeah, yuck.

@pietern
Copy link

pietern commented Jan 7, 2016

Working on a fix...

Not overly proud of the following snippet, but I'm using it as a stop gap until there is a new version of GSON is released with a real fix:

for (int i = 1;; i++) {
    try {
        return gson.toJson(obj);
    } catch (IllegalStateException e) {
        if (e.getMessage() == null) {
            if (i >= MAX_SERIALIZATION_ATTEMPTS) {
                throw e;
            }

            // This error may happen when two threads try to serialize a recursive
            // type for the very first time concurrently. Type caching logic in GSON
            // doesn't deal well with recursive types being generated concurrently.
            // Also see: https://github.com/google/gson/issues/764
            Thread.yield();
            continue;
        }

        throw e;
    }
}

photon-gerrit pushed a commit to vmware-archive/xenon that referenced this issue Jan 7, 2016
This error may happen when two threads try to serialize a recursive type
for the very first time concurrently. Type caching logic in GSON doesn't
deal well with recursive types being generated concurrently.

Also see: google/gson#764

Change-Id: I64babcd7f8b690ace59d4948eeeabc13a740b2f1
ktmprabhu pushed a commit to ktmprabhu/xenon that referenced this issue Mar 14, 2016
This error may happen when two threads try to serialize a recursive type
for the very first time concurrently. Type caching logic in GSON doesn't
deal well with recursive types being generated concurrently.

Also see: google/gson#764

Change-Id: I64babcd7f8b690ace59d4948eeeabc13a740b2f1
tibor-universe pushed a commit to getuniverse/gson that referenced this issue Dec 15, 2016
This race happens when two or more threads try to serialize the same
recursive type for the first time at the same time.

The fix makes sure that TypeAdapter instances are not cached in the
instance cache until all FutureTypeAdapters in a re-entrant call to
getAdapter() have their delegates set.

Also see: google#764
yrodiere added a commit to yrodiere/hibernate-search that referenced this issue Oct 3, 2019
yrodiere added a commit to yrodiere/hibernate-search that referenced this issue Oct 3, 2019
We don't need multiple implementations anymore, and we'll need to
execute complex initialization code to work around a bug in Gson
(google/gson#764).
yrodiere added a commit to yrodiere/hibernate-search that referenced this issue Oct 3, 2019
yrodiere added a commit to yrodiere/hibernate-search that referenced this issue Oct 3, 2019
yrodiere added a commit to yrodiere/hibernate-search that referenced this issue Oct 3, 2019
We don't need multiple implementations anymore, and we'll need to
execute complex initialization code to work around a bug in Gson
(google/gson#764).
yrodiere added a commit to yrodiere/hibernate-search that referenced this issue Oct 3, 2019
fax4ever pushed a commit to hibernate/hibernate-search that referenced this issue Oct 3, 2019
fax4ever pushed a commit to hibernate/hibernate-search that referenced this issue Oct 3, 2019
We don't need multiple implementations anymore, and we'll need to
execute complex initialization code to work around a bug in Gson
(google/gson#764).
fax4ever pushed a commit to hibernate/hibernate-search that referenced this issue Oct 3, 2019
@Marcono1234
Copy link
Collaborator

Same as #625

@Marcono1234
Copy link
Collaborator

This should be fixed by #1832, which will be available in the next release

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants