165

Here's the model I implemented:

public class LoginSession {
    private static final Gson gson = new Gson();

    private String id;
    private String name;
    private long timestamp;

    public LoginSession(String id, String name) {
        this.id = id;
        this.name = name;
        this.timestamp = System.currentTimeMillis();
    }

    public String toJson() {
        return gson.toJson(this);
    }

    public static LoginSession fromJson(String json) {
        checkArgument(!isNullOrEmpty(json));
        return gson.fromJson(json, LoginSession.class);
    }
}

I thought it's useless to create new Gson instance for every LoginSession instance.

But what I'm worried about is thread-safety issues. Approximately 1000+ instances/sec will be created.

Is it OK to use Gson instance as static field?

Thanks for any advices/corrections.

Christophe Roussy
  • 16,299
  • 4
  • 85
  • 85
philipjkim
  • 3,999
  • 7
  • 35
  • 48

4 Answers4

151

It seems just fine to me. There is nothing in the GSON instance that makes it related to a specific instance of LoginSession, so it should be static.

GSON instances should be thread-safe, and there was a bug regarding that which was fixed.

MByD
  • 135,866
  • 28
  • 264
  • 277
  • @slott, how do you guys pool/reuse Gson instances? Do you instantiate one every time you need to serialize? Or use a threadlocal pool? – Dilum Ranatunga Oct 29 '13 at 19:19
  • We use GSON together with Google Volley and when we parse JSON data concurrent we see this problem. From what I can see this is related to the fact that we define a timestamp for parsing datetime values. – slott Oct 30 '13 at 13:15
  • 2
    Datetime is not thread safe, that can be the cause, not that GSON is not thread safe. – Andreas Mattisson Nov 29 '16 at 10:55
28

The core Gson class is thread-safe. I just encountered a thread-safety issue that was supposedly with GSON. The issue happened when using a custom JsonDeserializer and JsonSerializer for Date parsing and formatting. As it turned out, the thread-safety issue was with my method's use of a static SimpleDateFormat instance which is not thread-safe. Once I wrapped the static SimpleDateFormat in a ThreadLocal instance, everything worked out fine.

entpnerd
  • 10,049
  • 8
  • 47
  • 68
  • 5
    A better option may be to use Apache commons FastDateFormat (part of commons-lang), which is explicitly threadsafe. https://commons.apache.org/proper/commons-lang/apidocs/org/apache/commons/lang3/time/FastDateFormat.html – Marceau Oct 03 '16 at 08:51
  • Thanks @Zaan. Great tip! – entpnerd Oct 03 '16 at 17:24
  • 1
    I was using Gson and SimpleDateFormat and getting thread errors on the Gson.fromJson. It turns out the class I was converting to had a non-threadsafe HashMap. I switched it to ConcurrentHashMap and everything works perfectly. – Gregory Alan Bolcer Dec 31 '20 at 20:22
9

According to the comments the existing unit test does not really test much, be careful with anything related to thread safety...

There is a unit test checking for thread safety:

/**
 * Tests for ensuring Gson thread-safety.
 *
 * @author Inderjeet Singh
 * @author Joel Leitch
 */
public class ConcurrencyTest extends TestCase {
  private Gson gson;
  ...

You may wonder if this unit test is sufficient to find every possible problem on every possible machine configuration ? Any comments on this ?

There is also this sentence in the docs:

The Gson instance does not maintain any state while invoking Json operations. So, you are free to reuse the same object for multiple Json serialization and deserialization operations.

Vili
  • 1,599
  • 15
  • 40
Christophe Roussy
  • 16,299
  • 4
  • 85
  • 85
  • 3
    I would have said this unit test was woefully inadequate to detect concurrency issues. First, the MyObject is a trivial class with no complex collections involved so concurrent de/serialization of lists and maps and other complex objects are not tested. Second, the serialization is only iterated 10 times per each of the 10 threads, which is inadequate. Third, concurrency faults are notoriously difficult to test anyway because different hardware configurations have different runtime characteristics, so any test would only be valid if guaranteed to be run on all configurations. – Lawrence Dol Dec 04 '14 at 20:40
  • 1
    For example, this test will likely not find any concurrency fault on a single core machine, since each thread will probably complete within a single timeslice and the threads will therefore run consecutively, not concurrently. – Lawrence Dol Dec 04 '14 at 20:41
  • 3
    No to say it's not threadsafe, only that this test does not even remotely guarantee that it is. – Lawrence Dol Dec 04 '14 at 20:43
  • 1
    The question of "if this unit test is sufficient to find every possible problem on every possible machine configuration" is completely irrelevant. I think any set of tests would be impossible to find all possibly concurrency issues. That they have a test is a good start and then a new test should be created for every concurrency bug that is fixed. Also, this is open source if you want more tests, go and write them. – mjaggard Jul 11 '22 at 10:45
1

We had issues with thread safety a while back and we solved it by using the FastDateFormat in apache commons.

Just created a gist Link for Gist around this to help the people wondering if Gson instances can be reused. They do not have setters and all vars are private.

So other than the SimpleDateFormat issue I don't see them maintaining state anywhere else.

Do check it out. This is my first time replying to one of these . Happy to give back for once . :)

aarengee
  • 11
  • 1