Is SimpleDateFormat.clone() thread-safe?

Question

I'm looking at legacy code and found the following:

private static final SimpleDateFormat sdf = new SimpleDateFormat("...");
...
void foo() {
    bar(date, someMoreArgs, sdf.clone());
}

where bar() then goes ahead and uses the passed SimpleDateFormat to format the given date.

Is the above code thread-safe? If multiple threads concurrently call sdf.clone(), can one of the cloned objects end up getting corrupted?

I wouldn't write the code like that myself in the first place. I know there are better ways to do this. But I'm not looking to refactor the code unless it can be proven to be not thread-safe.

Edit:
Some more information for clarification:

1. The static object sdf itself is never used for formatting. The only operation it's ever used for is cloning. Thus, I'm not expecting its contents to change (unless the cloning operation writes some transient data inside the object).

2. The clone is never used by more than one thread.

Answer 1

From 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.

So I believe that it depends on how you use that clone, but it is not assured. Cloning does not make your classes thread-safe. If the cloned object is not shared between class instances it should work with no problems, but I would not recommend this approach. However, you need a thread-safe date formatter I would suggest using Apache Commons FastDateFormat, described here.

Answer 2

Basically the clone() method doesn't give you thread safety. It just copies the properties of one object to another one. It doesn't lock or synchronize that object so if it is thread safe or not is up to the implementation. If during that copy some of the properties of the original object are changed then you might get into a strange state. If you use that cloned object in more than one threads - you still got problems

For your particular example I think the code is fine. The sdf object you are going to clone is probably never gonna change and you don't need a lock or something (it seems). You just create a new SimpleDateFormat object for each thread to ensure thread safety - or at least that's the idea - and you achieve that by using clone.

Anyway if you have spotted a problem in legacy code and you don't like it it is always better to spend some time and refactor it than to keep it like that even if you don't like it. It almost always pays off in the long term with having better and more maintainable code and not leaving that like that for the next developer to wander. For example if you have upgraded to java 8 you can use DateTimeFormatter which is thread safe or you can use some external library. Or at least create a new SimpleDateFormat(SOME_CONSTANT_FORMAT) everytime you need one instead of relying on clone of the object - because if you share just a string constant (the actual format) it is immutable and thread safe.

Answer 3

This is not an answer. But some information you may still find interesting. I did a couple of experiments.

First I had two threads formatting dates using the same SimpleDateFormat instance. After a couple of iterations they began giving incorrect results, and after a few hundred iterations one of the threads crashed. So the thread-unsafety seems very real.

Next I had one thread format dates using the original SimpleDateFormat and the other one taking clones of it and using the clones for formatting. Both threads have run for several minutes now and are still both producing correct results.

This is by no means any guarantee that this is the behaviour you will always see. The documentation is pretty clear: SimpleDateFormat is not thread safe and all access from several threads must be synchronized. So use the information at your own risk.

EDIT: Inspecting the source code seems to reveal that the clone operation copies fields in some order, but doesn’t modify the original. If the original was doing any work in another thread, this might cause the clone to be in an inconsistent state after creation, which in turn might or might not affect its correct working. If the original is only used for cloning, I see no risk with the current implementation. As you say, the implementation may be changed in later Java versions, but I would consider the risk small, and the risk of thread-unsafe behaviour being introduced even smaller. All of this is pure speculation!