Name: dbT83986 Date: 04/09/99


This is a resubmit of 4146524.

The response to 4146524 assumes that the solution to SimpleDateFormat's lack of thread-safety is synchronization. That assumption is incorrect. Synchronization would only be a bandage.

The cause of the problem is that a value (the date portion of calendar) used only for the duration of a single public method (parse) is stored as a member variable. (This design flaw was inspired by another design flaw of a similar nature in Calendar, but let's not go there.)

The solution to this problem is simple:

- at the beginning of parse, clone the calendar member and perform the mutating operations on the clone.

- pass the cloned calendar to all the methods used by parse that reference the calendar.

Aside from the obvious, there are two reasons why this fix should be made:

- The documentation for DateFormat states that a DateFormat object should be used multiple times, implying that construction is expensive. Furthermore, no mention is made of SimpleDateFormat's lack of thread-safety. Since for most applications the date formats remain constant, it is very easy to conclude that DateFormats should be application globals. But SimpleDateFormat produces incorrect results when used in this way.

- Bug 4101500, a similar problem with NumberFormat, was fixed.

===========================
REVIEW NOTE 4/15/99 - User sent additional info

I'd like to make one clarification: the problem is
in two public methods (parse and format), rather than the single method I
identified in the post. The post applies to both methods equally
(Review ID: 56749)
======================================================================

Name: tb29552 Date: 11/15/99


java version "1.2.2"
Classic VM (build JDK-1.2.2-W, native threads, symcjit)


DateFormat (and SimpleDateFormat) are not thread-safe.

Either they should be thread-safe, or the documentation should (clearly) state
that they are not.

---------- Test output:
Testing with DateFormat object
Thread[Tester2,5,main] got Nov 0015, 1999, expected Nov 15, 1999
Thread[Tester1,5,main] got Nov 15, 1999, expected Dec 31, 1969
Thread[Tester2,5,main] got Nov 15, 1969, expected Nov 15, 1999
Thread[Tester2,5,main] got Dec 31, 1969, expected Nov 15, 1999
Thread[Tester1,5,main] got Dec 0031, 1969, expected Dec 31, 1969
Thread[Tester2,5,main] got Nov 15, 1969, expected Nov 15, 1999
Thread[Tester1,5,main] got Dec 0031, 1969, expected Dec 31, 1969
Thread[Tester2,5,main] got Dec 31, 1969, expected Nov 15, 1999
...
Thread[Tester1,5,main] got Dec 0031, 1999, expected Dec 31, 1969
Thread[Tester1,5,main] got Dec 15, 1999, expected Dec 31, 1969
java.lang.IllegalArgumentException
at java.util.SimpleTimeZone.getOffset(SimpleTimeZone.java, Compiled
Code)
at java.util.GregorianCalendar.computeFields(GregorianCalendar.java,
Compiled Code)
at java.util.Calendar.setTimeInMillis(Calendar.java, Compiled Code)
at java.util.Calendar.setTime(Calendar.java, Compiled Code)
at java.text.SimpleDateFormat.format(SimpleDateFormat.java, Compiled
Code)
at java.text.DateFormat.format(DateFormat.java, Compiled Code)
at test.TestDateFormat$Tester.run(TestDateFormat.java, Compiled Code)
Thread1.failures = 192 Thread2.failures = 203

----------- TestDateFormat.java
package test;

import java.util.*;
import java.text.*;

/**
 * Demonstrate that java.text.SimpleDateFormat is not thread-safe.
 *
 * @version $Revision: 22$
 *
 * @author <a href="mailto:###@###.###">Kevin J. Butler</a>
 */
public class TestDateFormat
{
    public static boolean stopping = false;
    
    static class Tester extends Thread
    {
        DateFormat dateFormat;
        Date date;
        String expected;
        int failures = 0;
        
        public Tester( String name, DateFormat df, Date d )
        {
            super( name );
            this.dateFormat = df;
            this.date = d;
            this.expected = df.format( d );
        }

        public void run()
        {
            while ( !stopping )
            {
                String newText = dateFormat.format( date );
                if ( !newText.equals( expected ))
                {
                    failures++;
                    System.err.println(
                        Thread.currentThread() + " got " + newText +
                        ", expected " + expected
                        );
                }
            }
        }
    }

    public static boolean testDateFormat(
        DateFormat dateFormat
        )
    {
        Date d1 = new Date( 0 );
        Date d2 = new Date();

        Tester t1 = new Tester( "Tester1", dateFormat, d1 );
        Tester t2 = new Tester( "Tester2", dateFormat, d2 );

        t1.start();
        t2.start();
        try
        {
            Thread.sleep( 5000 );
            stopping = true;
            t1.join();
            t2.join();
        }
        catch ( InterruptedException ex )
        {
        }
        System.err.println( "Thread1.failures = " + t1.failures +
                            " Thread2.failures = " + t2.failures
                            );
        return t1.failures == 0 && t2.failures == 0;
    }

    public static void main( String[] args )
    {
        DateFormat dateFormat = DateFormat.getDateInstance();
        System.out.println( "Testing with DateFormat object" );
        testDateFormat( dateFormat );
// DateFormat synchronizedDateFormat =
//
com.pipeline.util.SynchronizedDateFormat.getSynchronizedDateFormat( dateFormat
);
// System.out.println( "Testing with SynchronizedDateFormat object" );
// testDateFormat( synchronizedDateFormat );
    }
}
(Review ID: 97854)
======================================================================