Skip to content

Conversation

@ryandotsmith
Copy link
Contributor

@ryandotsmith ryandotsmith commented Jun 21, 2018

It seems that our serializer is not safe to use across threads. This patch removes our global serializer object and adds a serializer instance variable on the Client object. If folks instantiate client objects across threads, then we should be ok here.

Example stack trace from a customer:

Caused by: java.util.ConcurrentModificationException
    at java.util.ArrayList$Itr.checkForComodification(ArrayList.java:901)
    at java.util.ArrayList$Itr.next(ArrayList.java:851)
    at com.chain.google.gson.internal.bind.CollectionTypeAdapterFactory$Adapter.write(CollectionTypeAdapterFactory.java:96)
    at com.chain.google.gson.internal.bind.CollectionTypeAdapterFactory$Adapter.write(CollectionTypeAdapterFactory.java:61)
    at com.chain.google.gson.internal.bind.TypeAdapterRuntimeTypeWrapper.write(TypeAdapterRuntimeTypeWrapper.java:68)
    at com.chain.google.gson.internal.bind.MapTypeAdapterFactory$Adapter.write(MapTypeAdapterFactory.java:208)
    at com.chain.google.gson.internal.bind.MapTypeAdapterFactory$Adapter.write(MapTypeAdapterFactory.java:145)
    at com.chain.google.gson.Gson.toJson(Gson.java:661)
    at com.chain.google.gson.Gson.toJson(Gson.java:640)
    at com.chain.google.gson.Gson.toJson(Gson.java:595)
    at com.chain.google.gson.Gson.toJson(Gson.java:575)
    at com.chain.http.Client.post(Client.java:342)
    at com.chain.http.Client.singletonBatchRequest(Client.java:235)
    at com.chain.signing.HsmSigner.sign(HsmSigner.java:70)

Related issue in gson: google/gson#1159

@ryandotsmith ryandotsmith self-assigned this Jun 21, 2018
@ryandotsmith ryandotsmith requested a review from dominic June 21, 2018 23:10
Copy link
Contributor

@dominic dominic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@ryandotsmith ryandotsmith merged commit 4e9b90d into main Jun 21, 2018
@ryandotsmith ryandotsmith deleted the ryan/java-fix-concur branch June 21, 2018 23:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants