Unexpected behavior of static member in multithread processing

Question

I have solved an issue about multithread processing at random. I'm happy because it works but I would like to know why. The faulty member in the code below is called INPUT_SDF. I thought static final members didn't need synchronized block but when I remove it, everything goes wrong.

public class A implements Comparable<A>
{

    public static final SimpleDateFormat INPUT_SDF  = new SimpleDateFormat("EEE MMM dd yyyy HH:mm:ss", Locale.US);

    ...

    public void setDate(String string) throws ParseException
    {
        synchronized (INPUT_SDF)
        {
            date = INPUT_SDF.parse(string);
        }
    }

}

Is my understanding of static final members wrong ? Or is there something else in my code not thread-safe ?

If you are not stuck with pre-8 Java, then I warmly advise the new Date API, which includes thread-safe parsers and formatters. — Marko Topolnik, Sep 17 '14 at 08:13

Chris K · Answer 1 · 2014-09-17T08:21:47.623

No making fields static final is not enough to make code thread safe. It only makes the assignment of the reference to an object thread safe, and thus ensures that other threads will see the same object reference within that field. It does not make mutation of the data stored within the object that was stored in the field (after the assignment) thread safe. Which is the problem with SimpleDataFormat.

From the documentation of SimpleDateFormat.

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.

If SimpleDateFormat had been stateless, or written to have its state to be thread safe then yes static final would have been enough to be thread safe.

As it is, if one wants to share the same instance of SimpleDataFormat between threads, one must synchronize the threads against the same monitor first. Otherwise creating a new instance of SimpleDateFormat per thread is recommended, either create a new instance as required or use ThreadLocal or a similar mechanism.

score 2 · Answer 2 · edited May 23 '17 at 10:26

2

Class SimpleDateFormat is not synchronized.

You can take a look to this question for more info. Hope it helps.

edited May 23 '17 at 10:26

Community

1
1

answered Sep 17 '14 at 08:15

troig

7,072
4
37
63

Bohemian · Accepted Answer · 2014-09-17T15:47:31.607

Making a field static final only makes the reference threadsafe; it doesn't synchronize access to the referenced object.

Because instances of SimpleDateFormat are not threadsafe, if an instance is used concurrently by multiple threads, you must synchronize access to it (as you did).

Making the field static final just guaranteed that all threads would see the same value for the reference.

However, your current code is a potential bottleneck, as all threads must queue up to use the instance, possibly yielding worse performance than just creating a new one each time.

If you want to safely reuse instances of SimpleDateFormat, consider using ThreadLocal to conveniently allow a separate instance to be using for each thread, which would allow you to remove synchronization:

private static final ThreadLocal<SimpleDateFormat> formats =
    new ThreadLocal<SimpleDateFormat>() {
        @Override protected SimpleDateFormat initialValue() {
            return new SimpleDateFormat("EEE MMM dd yyyy HH:mm:ss", Locale.US);
        }
    };

public void setDate(String string) throws ParseException {
    return formats.get().parse(string);
}

Thanks a lot Bohemian, I felt the bottleneck effect but didn't know it. Please allow me to correct a small mistake in your example code : you need to replace the last '}' of field formats by a ';'. — Bludwarf, Sep 17 '14 at 12:39
@Bludwarf indeed - thx for catching that. In my defence, I thumbed that code straight into the answer using my iphone while on the train. — Bohemian, Sep 17 '14 at 15:50

Unexpected behavior of static member in multithread processing

3 Answers3