5

I want to show to a colleague that SimpleDateFormat is not thread-safe through a simple JUnit test. The following class fails to make my point (reusing SimpleDateFormat in a multi-threaded environment) and I don't understand why. Can you spot what is preventing my use of SDF from throwing a runtime exception?

public class SimpleDateFormatThreadTest
{
    @Test
    public void test_SimpleDateFormat_MultiThreaded() throws ParseException{
        Date aDate = (new SimpleDateFormat("dd/MM/yyyy").parse("31/12/1999"));
        DataFormatter callable = new DataFormatter(aDate);

        ExecutorService executor = Executors.newFixedThreadPool(1000);
        Collection<DataFormatter> callables = Collections.nCopies(1000, callable);

        try{
            List<Future<String>> futures = executor.invokeAll(callables);
            for (Future f : futures){
                try{
                    assertEquals("31/12/1999", (String) f.get());
                }
                catch (ExecutionException e){
                    e.printStackTrace();
                }
            }
        }
        catch (InterruptedException e){
            e.printStackTrace();
        }
    }
}

class DataFormatter implements Callable<String>{
    static SimpleDateFormat sdf = new SimpleDateFormat("dd/MM/yyyy");

    Date date;

    DataFormatter(Date date){
        this.date = date;
    }

    @Override
    public String call() throws RuntimeException{
        try{
            return sdf.format(date);
        }
        catch (RuntimeException e){
            e.printStackTrace();
            return "EXCEPTION";
        }
    }
}
Bharat Sinha
  • 13,973
  • 6
  • 39
  • 63
  • 1
    You should tell your colleague to read the javadoc so you can get on with some proper work! – David Grant Sep 21 '12 at 15:20
  • Yes SimpleDateFormat is not thread safe and our (bad) experience with it, is that it does not throw Exceptions, it performs crazy formatting (which is actually worse). Try outputting values in your code. – Bruno Grieder Sep 21 '12 at 15:21
  • Create another SDF for each thread and format it both ways. Then compare the output and throw an exception if the output doesnt match. – Stefan Sep 21 '12 at 15:27
  • 2
    The only way to prove code is or is not thread safe is to read the code. There is no way to ensure you have triggered all possible bugs by running a unit test. – Peter Lawrey Sep 21 '12 at 15:38
  • Absolutely. It boggles my mind that somebody would ask that something like this be proven to them. I suppose I could see writing a test to try and force it to malfunction, if that's the kind of thing you enjoy doing. Everybody needs a hobby. It IS possible (though difficult) to prove something is thread unsafe by testing: you can demonstrate it and eliminate all the thread-safe elements, leaving only one possibility. But I think what we care more about is proving thread safety, which you cannot do with testing. – fool4jesus Jan 16 '13 at 00:38
  • Thread safety issues are by their nature non-deterministic. You could test something 10,000 times without seeing any problems, only for it fail on the 10,001st try. If some code does not say that it definitely _is_ threadsafe then you have to assume it isn't. – Ian Roberts Feb 07 '14 at 22:16

3 Answers3

13

Lack of thread safety doesn't necessarily mean that the code will throw an exception. This was explained in Andy Grove's article, SimpleDateFormat and Thread Safety, which is no longer available online. In it, he showed SimpleDateFormat's lack of thread safety by showing that the output would not always be correct, given different inputs.

When I run this code, I get the following output:

    java.lang.RuntimeException: date conversion failed after 3 iterations.
    Expected 14-Feb-2001 but got 01-Dec-2007

Note that "01-Dec-2007" isn't even one of the strings in the test data. It is actually a combination of the dates being processed by the other two threads!

While the original article is no longer available online, the following code illustrates the issue. It was created based on articles that appeared to have been based on Andy Grove's initial article.

import java.text.ParseException;
import java.text.SimpleDateFormat;
import java.util.Date;
import java.util.List;
import java.util.Locale;

public class SimpleDateFormatThreadSafety {
    private final SimpleDateFormat dateFormat = new SimpleDateFormat("dd-MMM-yyyy", Locale.US);

    public static void main(String[] args) {
        new SimpleDateFormatThreadSafety().dateTest(List.of("01-Jan-1999", "14-Feb-2001", "31-Dec-2007"));
    }

    public void dateTest(List<String> testData) {
        testData.stream()
                .map(d -> new Thread(() -> repeatedlyParseAndFormat(d)))
                .forEach(Thread::start);
    }

    private void repeatedlyParseAndFormat(String value) {
        for (int i = 0; i < 1000; i++) {
            Date d = tryParse(value);
            String formatted = dateFormat.format(d);
            if (!value.equals(formatted)) {
                throw new RuntimeException("date conversion failed after " + i
                        + " iterations. Expected " + value + " but got " + formatted);
            }
        }
    }

    private Date tryParse(String value) {
        try {
            return dateFormat.parse(value);
        } catch (ParseException e) {
            throw new RuntimeException("parse failed");
        }
    }
}

Sometimes this conversion fails by returning the wrong date, and sometimes it fails with a NumberFormatException:

java.lang.NumberFormatException: For input string: ".E2.31E2"
M. Justin
  • 14,487
  • 7
  • 91
  • 130
Claudiu
  • 224,032
  • 165
  • 485
  • 680
  • It is possible for it to throw exceptions, but they'll be disguised as strange parsing exceptions, like the one discussed [here](http://www.coderanch.com/t/569795/java/java/SimpleDateFormat-not-thread-safe). – Brian Sep 21 '12 at 15:22
  • I read the article and updated my code accordingly. But still cannot see this behaviour on my JUnit. Is it something I misunderstand with multithreading? –  Sep 21 '12 at 15:39
  • @Pomario: maybe try giving it different dates to format at the same time? – Claudiu Sep 21 '12 at 15:45
  • @Pomario, maybe you should try to switch to another architecture that has more CPUs/cores to run your tests. – Gray Sep 21 '12 at 17:55
  • Unfortunately, the link is now dead. – M. Justin Sep 11 '20 at 22:44
  • As the link is now dead, I've pieced together what I believe the original code originally did from various mostly articles (in languages I don't speak) that appear to have been based on the original blog post. I've updated the answer with this code I've created based on it. – M. Justin Sep 13 '20 at 04:37
4

Isn't this part from javadoc of SimpleDateFormatter has sufficent proof about it?

Synchronization

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.

And the major observation of not being thread safe is to get unexpected results and not an exception.

Community
  • 1
  • 1
Bharat Sinha
  • 13,973
  • 6
  • 39
  • 63
3

It is not thread safe because of this code in SimpleDateFormat (in sun JVM 1.7.0_02):

private StringBuffer format(Date date, StringBuffer toAppendTo,
                            FieldDelegate delegate) {
    // Convert input date to time field list
    calendar.setTime(date);
    ....
}

Each call to format stores the date in a calendar member variable of the SimpleDateFormat, and then subsequently applies the formatting to the contents of the calendar variable (not the date parameter).

So, as each call to format happens the data for all currently running formats may change (depending on the coherence model of your architecture) the data in the calendar member variable that is used by every other thread.

So if you run multiple concurrent calls to format you may not get an exception, but each call may return a result derived from the date of one of the other calls to format - or a hybrid combination of data from many different calls to format.

lexicalscope
  • 7,158
  • 6
  • 37
  • 57