Is this variable being safely accessed by using sy

问题:

I have a case in which a Java class has a superclass that contains a synchronized block.

Class SuperClassA {
     private Bitmap bmpA;

     protected abstract Bitmap createBitmap();

     public void run() {
          synchronized (this) {
               bmpA = createBitmap();
          }
     }

     // some other codes.
}

Class SubClassB extends SuperClassA {
     private Bitmap outBmpB;

     protected Bitmap createBitmap() {
          outBmpB = ..... // create and process "outBmpB".

          Bitmap bmp;
          bmp = ..... // create and process "bmp".
          return bmp;
     }

     public Bitmap getOutBmpB() {
          Bitmap tempBmp;
          synchronized (this) {
               tempBmp = outBmpB.clone();
          }
          return tempBmp;
     }

     // some other codes.
}

The "getOutBmpB()" method in Class B is run by a thread while the inherited "run()" method in ClassB is run by another thread. The "createBitmap()" method implemented in ClassB should be run in a synchronized block inside the "run()" method.

My question is that I am not sure whether the newly defined class variable "outBmpB" in ClassB is safely accessed by the two threads. I am not sure the "synchronized (this)" block in the "run()" method would also "lock" the "outBmpB" variable defined just in ClassB ? If not, then could I add a "synchronized (this)" block in the "createBitmap()" implementation. e.g.

 protected Bitmap createBitmap() {
      synchronized (this) {
           outBmpB = ..... // create and process "outBmpB".
      }

      Bitmap bmp;
      bmp = ..... // create and process "bmp".
      return bmp;
 }

Thanks for any suggestion.

Lawrence

回答1:

You don't have to synchronize it since it is already synchronized in super class and there are no other calls, but you should. Your implementation of createBitmap() relies on implementation details from the super class. Do synchronization at every point you access your shared fields.

Due to this facts your current subclass' code is very error-prone!

While it is questionable to ever synchronize over this, it is a much better idea to synchronize over a private object nobody else can access. So your code cannot be broken by clients using your class objects and synchronize on it.

回答2:

It's not about "locking" a variable: you just cannot do it. You do have a proper lock in your code. However, "thread safety" means, "It's safe for any reasonable usage". As your method changing outBmpB is protected—that is, available to the package members and all the children,—you're better off explicitly synchronizing.

As a general rule, every time you access shared mutable state, you must be sure you hold the lock. In your case, you are not sure. At the same time, Java locks are reentrant, so nested synchronization will not hurt—and will perhaps save you from surprises.

回答3:

As a general idea, it's better to lock on the object you are using (bmpA and outBmpB in your case) than on the container object, unless you really need to do that (and in that case a synchronized method would be better).

As per your particular case, the synchronized (this) blocks will exclude each other and so yes, it would work if you don't call createBitmap() anywhere else, and so it's far better to synchronize inside createBitmap() than inside run().