Thread safe access to private field

Question

So I have the following scenario (can't share the actual code, but it would be something like this):

public class Test
{
   private Object obj;

   public void init()
   {
       service.registerListener(new InnerTest());
   }

   public void readObj()
   {
       // read obj here
   }

   private class InnerTest implements Listener
   {
       public synchronized void updateObj()
       {
           Test.this.obj = new Object();
           // change the obj
       }
   }
}

The InnerTest class is registered as listener in a service. That Service is running in one thread the calls to readObj() are made from a different thread, hence my question, to ensure consistency of the obj is it enough to make the UpdateObj() method synchronized?

Possible duplicate of [Java synchronized method lock on object, or method?](https://stackoverflow.com/questions/3047564/java-synchronized-method-lock-on-object-or-method) — kabanus, Jul 03 '18 at 08:03
While you are guaranteed that only one Thread at once enters your `updateObj` method it is not locked itself. There is still a chance someone also gets hold of the Object before you are done applying changes, potentially creating a race condition. Lino's answer is one of the possible ways to assure this is not an issue. — Ben, Jul 03 '18 at 08:13

Lino · Accepted Answer · 2018-07-03T08:19:34.957

3

I would suggest using another object as a lock to ensure that the class only blocks when the obj is accessed:

public class Test
{
   private final Object lock = new Object();
   private Object obj;

   public void init()
   {
       service.registerListener(new InnerTest());
   }

   public void readObj()
   {
       synchronized(lock){
           // read obj here
       }
   }

   private class InnerTest implements Listener
   {
       public void updateObj()
       {
           synchronized(Test.this.lock){
               Test.this.obj = new Object();
               // change the obj
           }
       }
   }
}

Then use that lock in all methods that need to have consistent access to obj. In your current example the readObj and updateObj methods.

Also as stated in the comments, using synchronized on the method level in your InnerTest class, will not really work as you probably intended. That is, because synchronized methods will use a synchronized block on the this variable. Which just blocks your InnerTest class. But not the outer Test class.

edited Jul 03 '18 at 08:19

answered Jul 03 '18 at 08:08

Lino

19,604
6
47
65

2

I would suggest expanding the answer by explaining why synchronizing on the inner class instance is not helpful. – RealSkeptic Jul 03 '18 at 08:16
@RealSkeptic I didn't even think about that. I edited it into the answer, thanks. – Lino Jul 03 '18 at 08:20
Great explanation! Many thanks, before I accept that answer one last question though, is there any incentive/difference in using synchronized as you described vs i.e. Reentrant lock? – Stugal Jul 03 '18 at 08:22
1

@Stugal It really depends on the use case. Can the class be used without the `obj` or is it really dependant on it? In the first case the lock is the better choice. in the second one you'd have to block every call with synchronized methods and in the inner class use `synchronized(Test.this)` as a lock. That way you'll synchronize every access that is made. And thus will have a consistent behaviour – Lino Jul 03 '18 at 08:24

Thread safe access to private field

1 Answers1