8

For example:

static private DateFormat df = new SimpleDateFormat();
public static void format(final Date date) { 
   for (int i = 0; i < 10; i++) 
     new Thread(new Runnable() {
         public void run() {
             System.out.println(df.format(date));
         } 
     });
}

The DateFormat class is documented as not a synchronized class, but if we use just the format method, it can't change the status of the whole class?

Suppose it's declared private, how can one be sure that the code is thread safe?

What is the best way to fix this code?:

  1. Using a different instance for every thread.

  2. Using a synchronized block.

0xCursor
  • 2,242
  • 4
  • 15
  • 33
Nassim MOUALEK
  • 4,702
  • 4
  • 25
  • 44
  • 1
    By reading the documentation carefully, both the class documentation and the specific method documentation. – RealSkeptic Aug 15 '15 at 09:32
  • DateFormat especially mentions it's not thread safe if I'm not mistaken. – Davio Aug 15 '15 at 09:33
  • Ok it is documented as not synchornized Class, if we use just the format Method, it can't change the statut of the hole Class? – Nassim MOUALEK Aug 15 '15 at 09:39
  • It is specifically documented as *not synchronized". – RealSkeptic Aug 15 '15 at 09:40
  • The hole class is documented not safe but how to know if a specifique method chage the state of the Class or not ? – Nassim MOUALEK Aug 15 '15 at 09:43
  • 4
    If a class is documented as non threadsafe, like DateFormat is, you stop speculating about which method might or might not actually be thread-safe. You just don't share the instance between several threads. BTW, the fact that format() changes or doesn't change the state is irrelevant: if another method is called concurrently, and that method changes the state, then format() will have its invariants broken, and could return a wrong result, or throw an exception, or anything else. – JB Nizet Aug 15 '15 at 09:46
  • but if am sure that this instance is private, and it can't be used anyware else, it could be useful to know if the method dosn't change his state? – Nassim MOUALEK Aug 15 '15 at 09:48
  • 1
    It can be useful to read the source of a method and see how it is working, and the source is freely available on the Internet. However, you *cannot* rely on the implementation to stay the same through versions. And if your program doesn't allow any other threads to touch an object, why are you concerned about thread safety? – RealSkeptic Aug 15 '15 at 09:54
  • You could always write load test and see if data integrity is preserverd for your specific scenario. – John Aug 15 '15 at 09:55
  • i add a For iteration to the question – Nassim MOUALEK Aug 15 '15 at 09:56
  • BTW, format() does change the state of the formatter. It uses a Calendar instance variable and mutates it each time format() is called. – JB Nizet Aug 15 '15 at 10:10

3 Answers3

7
  • For a standard Java SE class, the best way to know whether or not the class is thread-safe is to carefully read its documentation. Always read both the class documentation and the method documentation. If either say it's not synchronized or not thread-safe, you know it's not thread-safe.
  • Therefore, the DateFormat class is not thread safe. The documentation specifically says:

    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.

  • Declaring a field private does not make your implementation thread-safe. private merely says that outside classes can't see that field. Let's look at your method:

     for (int i=0;i<10;i++) 
         new Thread(new Runnable(){
             public void run(){
                 System.out.println(df.format(date));
             } 
         });
    

    The Runnable objects that you create are anonymous classes. Anonymous classes are inner classes, which have access to private fields of their surrounding class. If it wasn't so, your program would not compile - they could not access the df field.

    But they can. So in fact you are having 10 threads that are all accessing your one DateFormat object, referred to by df. Since we already know that DateFormat is not thread-safe, your program is not thread-safe.

  • Furthermore, if two external threads have references to your object (I mean the object that has the df inside it. You didn't give the class declaration so I don't know what its name is). They have references to the same instance of your class. If both of them call format at the same time, both will be running DateFormat.format using the same private df. Thus, this is not going to be thread-safe.
  • To be thread-safe, you need to synchronize on the object or use some other kind of lock (one lock for all the possible threads that access it), which is exactly what the documentation said to do.
  • Another way is to have a completely local object, which is visible to only one thread. Not a field - a local variable, which has access to a uniquely created instance of DateFormat (so you have a new copy every time you call the method). Beware of anonymous classes, though! In your example, even if df was a local field to the format method, it would still not be thread-safe because all your threads would be accessing the same copy.
RealSkeptic
  • 33,993
  • 7
  • 53
  • 79
  • So even if the instance is local to the my method Format, and i am sure that all threads use only the Format method, which i suppose it is stateless, it still not thread safe? – Nassim MOUALEK Aug 15 '15 at 10:17
  • @NassimMOUALEK You can't *suppose*. This is still not thread safe. If the documentation says it's not, then it's not. More than one thread accessing the instance = not thread safe. – RealSkeptic Aug 15 '15 at 10:18
  • Yes but they dont specify about every method, they document the hole class as not synchorinized, but i am pretty sure that format date should be stateless – Nassim MOUALEK Aug 15 '15 at 10:20
  • it would be thread safe if you knew it was stateless. Until DateFormat.format implementation is changed, that is. – John Aug 15 '15 at 10:20
  • Again, you can't *suppose* or *assume* anything, @NassimMOUALEK. Since the class documentation says the class is not synchronized, it means any method is subject to implementation changes and can be implemented differently by different Java environment, and all those implementations *may* change state variables, hence, it's **not** thread-safe. – RealSkeptic Aug 15 '15 at 10:22
  • the best way to fix this code is to use a different instance for every Thread, or we should use a synchronized block ? which is better? probably the first solution i think – Nassim MOUALEK Aug 15 '15 at 10:48
  • 1
    In this particular case I'd personally go with a synchronized block, as `System.out.println` is going to be slower anyway, and involves synchronization as well, so the delay for synchronization is going to be negligible. Besides, there is also a price for making copies, which is usually justifiable only if the thread uses the instance many times. – RealSkeptic Aug 15 '15 at 10:56
  • You mean System.out.println has its own synchronisation block, or we must use a synchronisation block for every System.out.println ? – Nassim MOUALEK Aug 15 '15 at 11:04
  • 1
    I mean it has its own synchronization block. – RealSkeptic Aug 15 '15 at 11:06
3

As per the docs, it is stated that the format is not thread safe.

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.

Date format

How to read this ? If you don't have explicit guarantee that some method is thread safe(or its documented as unsafe) then you cannot make any assumptions about it being safe.

However, if you really wish to only use single method which might not be statefull, you can always create high concurrency environment and test for data integrity with and without synchronization.

I have had similar question to this, with Ciphers and RSA. The answer there shows one way how to test specific method of java SE class in general for this. Note however that implementation could change at any point, and by making your own implementation against implementation details rather then interface might cause some unpredictable issues in the future.

testing for data integrity

Community
  • 1
  • 1
John
  • 5,189
  • 2
  • 38
  • 62
2

I know it is hard to believe, but DateFormat.format() actually modifies the DateFormat's state. For instance, for SimpleDateFormat:

// Called from Format after creating a FieldDelegate
private StringBuffer format(Date date, StringBuffer toAppendTo,
                            FieldDelegate delegate) {
    // Convert input date to time field list
    calendar.setTime(date);

where calendar is a field of DateFormat.

Therefore, I can only recommend you trust the documentation. It may know things you don't.

meriton
  • 68,356
  • 14
  • 108
  • 175