Is nesting of locks necessary for this code?

I have two shared mutable objects in a class whose thread policy is defined as "thread safe".

public static final GregorianCalendar CAL = new GregorianCalendar();
public static final SimpleDateFormat  SDF = new SimpleDateFormat();

      

The goal is to reduce the amount of object creation, since these objects are expensive to create and the methods that will need to use them are expected to be called frequently.

Here's one such (static factory) method:

    public static MJD ofTimeStampInZone(String stamp, String form, TimeZone tz) {

        double result;

        synchronized(lockCal) {
            synchronized(lockSdf) {
                CAL.setTimeZone(tz);
                SDF.setCalendar(CAL);
                SDF.applyPattern(form);

                try {
                    Date d = SDF.parse(stamp);
                    CAL.setTime(d);
                    result = (CAL.getTimeInMillis() / (86400.0 * 1000.0)) + 
                            POSIX_EPOCH_AS_MJD;
                } 
                catch (ParseException e) 
                    { throw new IllegalArgumentException("Invalid parsing format"); }
            }
        }
        return new MJD(result);
    }

      

I also set a policy for this class that lockCal

should always be received before lockSdf

. However, this applies to this class as well:

  • CAL can be locked and used without SDF, in which case SDF is not locked.
  • SDF is never used in a method unless CAL is used

Since SDF depends on CAL, I am wondering if for-only locking is sufficient lockCal

to prevent data inconsistency during concurrent access. This would allow me to drop blocking on SDF. In other words, thread safety is still guaranteed, with the above conditions, if I only use:

    public static MJD ofTimeStampInZone(String stamp, String form, TimeZone tz) {

        double result;

        synchronized(lockCal) {
                CAL.setTimeZone(tz);
                SDF.setCalendar(CAL);
                SDF.applyPattern(form);

                try {
                    Date d = SDF.parse(stamp);
                    CAL.setTime(d);
                    result = (CAL.getTimeInMillis() / (86400.0 * 1000.0)) + 
                            POSIX_EPOCH_AS_MJD;
                } 
                catch (ParseException e) 
                    { throw new IllegalArgumentException("Invalid parsing format"); }
        }
        return new MJD(result);
    }

      

+3


source to share


2 answers


If SDF

used only by a thread that has already acquired it lockCal

, it can only be accessed by one thread at a time, that is, it is thread safe even if you remove the lock on lockSdf

.



If you choose to rely on this observation, you must document it clearly, so future maintenance programmers don't start using it SDF

out synchronized (lockCal)

.

+1


source


Generally speaking, thread safety is not guaranteed here. Assuming the SDF should be protected by lockSDF, but changed under a different lock, another thread might not see the result of SDF changes if it only acquires lockSDF. But you have a policy: purchase lockCal before lockSdf. It looks like "view" solves the problem, but

1) It's hard to talk about thread safety

2) This makes lockSdf useless

Assuming SDF depends on CAL and CAL is guarded by lockCal, then it makes sense to also use lockCal to protect SDF.



Java Concurrency in Practice (Brian Goetz):

Summary of Part 1

...

Keep all variables in the invariant with the same locking.

...

0


source







All Articles