Reputation: 2794
I want to test a fix where SimpleDateFormat was used as a static class variable. I was getting
Previously my code was
public class Abc{
private static SimpleDateFormat dateformatter;
public static String method1(final Calendar calendar) {
String thePattern = "ddMMM";
dateformatter = new SimpleDateFormat(thePattern, Locale.US);
sPars = dateformatter.format(calendar.getTime());
//Something
}
public static String method2(final Calendar calendar) {
String thePattern = "ddMMyyyy";
dateformatter = new SimpleDateFormat(thePattern, Locale.US);
sPars = dateformatter.format(calendar.getTime());
//Something
}
}
For this one, I was getting below exception
java.lang.ArrayIndexOutOfBoundsException: 965
at sun.util.calendar.BaseCalendar.getCalendarDateFromFixedDate(BaseCalendar.java:454)
at java.util.GregorianCalendar.computeFields(GregorianCalendar.java:2333)
at java.util.GregorianCalendar.computeFields(GregorianCalendar.java:2248)
at java.util.Calendar.setTimeInMillis(Calendar.java:1140)
at java.util.Calendar.setTime(Calendar.java:1106)
at java.text.SimpleDateFormat.format(SimpleDateFormat.java:955)
at java.text.SimpleDateFormat.format(SimpleDateFormat.java:948)
at java.text.DateFormat.format(DateFormat.java:336)
Now I have changed it to changed to :
public class Abc{
public static String method1(final Calendar calendar) {
String thePattern = "ddMMM";
SimpleDateFormat dateformatter = new SimpleDateFormat(thePattern, Locale.US);
sPars = dateformatter.format(calendar.getTime());
//Something
}
public static String method2(final Calendar calendar) {
String thePattern = "ddMMyyyy";
SimpleDateFormat dateformatter = new SimpleDateFormat(thePattern, Locale.US);
sPars = dateformatter.format(calendar.getTime());
//Something
}
}
How can I test that the second one is a proper fix ? Is there any way in JUnits to test multithreading ?
Upvotes: 1
Views: 3273
Reputation: 3751
SimpleDateFormat is not thread safe, as stated in the javadoc :
Date formats are not synchronized. It is recommended to create separate format instances for each thread. If multiple threads access a format concurrently, it must be synchronized externally.
But here you were even messing with references because of :
dateformatter = new SimpleDateFormat(thePattern, Locale.US);
Since dateformatter was static, its content was shared between threads.
Creating a new instance locally (like you did afterwards) is a good way to go. Local instances aren't shared between threads and won't mess with multithreading.
Just don't forget to remove the private static SimpleDateFormat dateformatter;
to avoid confusion.
You can also use a ThreadLocal variable if you want to create only 1 instance per thread :
private static final ThreadLocal<SimpleDateFormat> dateformatter1 =
new ThreadLocal<SimpleDateFormat>() {
@Override protected SimpleDateFormat initialValue() {
return new SimpleDateFormat("ddMMM");
}
};
private static final ThreadLocal<SimpleDateFormat> dateformatter2 =
new ThreadLocal<SimpleDateFormat>() {
@Override protected SimpleDateFormat initialValue() {
return new SimpleDateFormat("ddMMyyyy");
}
};
public static String method1(final Calendar calendar) {
SimpleDateFormat dateformatter = dateformatter1.get();
sPars = dateformatter.format(calendar.getTime());
//Something
}
public static String method2(final Calendar calendar) {
SimpleDateFormat dateformatter = dateformatter2.get();
sPars = dateformatter.format(calendar.getTime());
//Something
}
Upvotes: 5