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

Thread issues with date formatter #162

Closed
GoogleCodeExporter opened this issue Mar 19, 2015 · 8 comments
Closed

Thread issues with date formatter #162

GoogleCodeExporter opened this issue Mar 19, 2015 · 8 comments

Comments

@GoogleCodeExporter
Copy link

What steps will reproduce the problem?
1.  Run high levels of threads which do date serialization
2.  DefaultDateTypeAdapter uses SimpleDateFormat statically
3.  See lots of exceptions on random occasions

What is the expected output? What do you see instead?

Expect date serialization to work!
 public JsonElement serialize(Date src, Type typeOfSrc,
JsonSerializationContext context) {
      String dateFormatAsString = format.format(src);
      return new JsonPrimitive(dateFormatAsString);
    }
Changing the constructor to:
 public DefaultDateTypeAdapter(final String datePattern) {
      this.format = new ThreadLocal<DateFormat>() {
        protected DateFormat initialValue() {
                   new SimpleDateFormat(datePattern);
                };
    }

  public JsonElement serialize(Date src, Type typeOfSrc,
JsonSerializationContext context) {
      String dateFormatAsString = format.get().format(src);
      return new JsonPrimitive(dateFormatAsString);
    }

  public Date deserialize(JsonElement json, Type typeOfT,
JsonDeserializationContext context)
        throws JsonParseException {
      if (!(json instanceof JsonPrimitive)) {
        throw new JsonParseException("The date should be a string value");
      }

      try {
        return format.get().parse(json.getAsString());
      } catch (ParseException e) {
        throw new JsonParseException(e);
      }
    }


Would be a simple fix.


What version of the product are you using? On what operating system?

Latest GSON release.  Issue is using the date formatter statically, as
SimpleDateFormat isn't thread safe, so you'll get random results with the
date format.  It will also randomly throw exceptions.  See stack trace below.

Please provide any additional information below.

java.lang.ArrayIndexOutOfBoundsException: -28   at
sun.util.calendar.BaseCalendar.getCalendarDateFromFixedDate(BaseCalendar.java:43
6)
at
java.util.GregorianCalendar.computeFields(GregorianCalendar.java:2081)  at
java.util.GregorianCalendar.computeFields(GregorianCalendar.java:1996)  at
java.util.Calendar.setTimeInMillis(Calendar.java:1066)  at
java.util.Calendar.setTime(Calendar.java:1032)  at
java.text.SimpleDateFormat.format(SimpleDateFormat.java:785)    at
java.text.SimpleDateFormat.format(SimpleDateFormat.java:778)    at
java.text.DateFormat.format(DateFormat.java:314)    at
com.google.gson.DefaultTypeAdapters$DefaultDateTypeAdapter.serialize(DefaultType
Adapters.java:254)
at 

Original issue reported on code.google.com by mcinto...@gmail.com on 25 Sep 2009 at 4:12

@GoogleCodeExporter
Copy link
Author

Good catch. We chose to instead just synchronize the serialize and deserialize 
methods. See r452

Original comment by inder123 on 25 Sep 2009 at 5:15

  • Changed state: Fixed
  • Added labels: Milestone-Release1.4

@GoogleCodeExporter
Copy link
Author

Original comment by inder123 on 25 Sep 2009 at 5:15

@GoogleCodeExporter
Copy link
Author

Note, the synchronization will fix the threading issues, but there is a 
performance
cost of that versus thread local variables.  In high performance/highly threaded
environments, using SimpleDateFormat has a cost.  Because of the way it's
implemented, with regular expressions, when you're running a lot of threads 
there is
definitely a potential for blocking that could be problematic.  I don't have 
any hard
and fast numbers at the moment, but it might be worth while to run some 
profiling
with at least 16 threads to make sure there aren't any performance issues.

Original comment by mcinto...@gmail.com on 28 Sep 2009 at 5:05

@GoogleCodeExporter
Copy link
Author

Yes, we thought about the performance issues regarding synchronization. 
However, the synchronization is 
specific to the default DateTypeAdapter so the impact will be if you have lots 
of dates getting serialized at the 
same time. That shouldn't be a problem in real world situations in my opinion. 
If you have a situation where this 
is a concern, please reopen the bug and provide details and we will take 
another look.

Original comment by inder123 on 28 Sep 2009 at 3:11

@ilyassoomro
Copy link

We have too many Thread blocked at below line due to synchronized keyword.
com.google.gson.DefaultTypeAdapters$DefaultDateTypeAdapter.serialize(java:265)

We have heavy usage of date format in gson.

@swankjesse
Copy link
Collaborator

@ilyassoomro you can probably install a custom date adapter that uses separate SimpleDateFormat instances per-thread rather than shared ones.

@swankjesse
Copy link
Collaborator

Also - this adapter might not be of the exact format you want, but it's threadsafe & doesn't use synchronized.
https://github.com/google/gson/blob/master/extras/src/main/java/com/google/gson/typeadapters/UtcDateTypeAdapter.java

@aarengee
Copy link

The perfomance hit might be an issue for us so we resorted to using an overrride by using our own Serde for util.date and sql.date.

Uses FastDateFormat instead of SimpleDateFormat to ensure you dont take a performance hit due to serial usage (synchronize write and read)

Have added some sample code in github gist.

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

No branches or pull requests

4 participants