Why is lockStatic volatile in Android?

Question

I'm trying to understand this Android implementation, but I just can't figure out why static field lockStatic is volatile, since its use is only through getLock method and it is a synchronized block, so there should be no concurrency or object publishing issues.

abstract public class WakefulIntentService extends IntentService {
    abstract protected void doWakefulWork(Intent intent);
    static final String NAME=
      "com.commonsware.cwac.wakeful.WakefulIntentService";
    static final String LAST_ALARM="lastAlarm";

    private static volatile PowerManager.WakeLock lockStatic=null;

    synchronized private static PowerManager.WakeLock getLock(Context context) {
       if (lockStatic == null) {
           PowerManager mgr = (PowerManager)context.getSystemService(Context.POWER_SERVICE);

           lockStatic=mgr.newWakeLock(PowerManager.PARTIAL_WAKE_LOCK, NAME);
           lockStatic.setReferenceCounted(true);
       }

       return(lockStatic);
    }

The way I see there would be only the need for volatile if lockStatic was allowed access in any other way, but its private and only method which deals with it is the getLock method. Is this just an assertion or there is other reasons behind it?

Thanks in advance for any help resolving this question.

WeakfullIntentService

Answer 1

In the code sample, volatile is not needed, because the variable is only accessed in that synchronized block.

However, synchronized might be a little too expensive, especially considering that majority invocations to that method simply read the variable and return. It feels too heavy to surround every read with synchronized. Therefore we have the so-called double-checked pattern, which avoids synchronized most of times

if(var==null)
    synchronized(lock)
        if(var==null)         // double-check
            var = something
return var

(OP's code sample however is not double-checked locking; it's all-time locking.)

In doubled-checked locking, usually var should be volatile -- but not always. If the object assigned to it is primitive (like int) or immutable (like String), non-volatile is OK.

And there are more situations where non-volatile is OK. This requires careful evaluations of the use cases. For example, all utility classes in java.util.concurrent (e.g. ReentrantLock) are designed in a way so that they will survive "unsafe publication". When they are used in double-checked locking pattern, they don't have to be volatile. Seeing the source code of WakeLock, I believe it doesn't need volatile either in double-checked locking.

Of course, this is a little too much. So, unless you know exactly what you are doing, play it safe and use volatile. A volatile read isn't that much expensive over an ordinary read.