3
private Object x = new Object();

public void doA() {
   synchronized(x){
       x = new Object();
       //do something
    }
}

public void doB() {
    synchronized(x) {
        //do something 
    }
}

lets say doA() and doB() was called simultaneously but doA() is proceeding first. so B will be blocked until doA() is completed

is this true even if doA() modifies x in the x = new x call? or after this line x = new x in doA() doB() will no longer be blocked because x has changed?

Kröw
  • 845
  • 1
  • 10
  • 26
ealeon
  • 10,271
  • 16
  • 74
  • 143
  • You can always try running this experiment on your local machine, possibly using long sleeps and print statements so that you can see things happening slowly enough to understand easily. – nanofarad May 23 '19 at 18:55
  • Lock is held on the original reference. Why would it change? – yegodm May 23 '19 at 18:58
  • @yegodm okay that makes sense – ealeon May 23 '19 at 18:59
  • what exactly is `x` ? a class? a field? (a constructor?) – user85421 May 23 '19 at 19:03
  • Regardless, the state that `//do something` affects is _eventually_ no longer guarded by the same lock and thus there's not a valid _happens-before_ relationship. – Slaw May 23 '19 at 19:03
  • @Slaw correct and thats a very good point to highlight but in my use case, that is fine because the state is ever-changing and it is expected to use what's-availalbe-now information even though it may be stale. I am trying to avoid ConcurrentModificationException – ealeon May 23 '19 at 19:17
  • Can you give us more details on the context? Synchronization is not only about avoiding ConcurrentModificationExceptions but also about visibility issues ... e.g. https://stackoverflow.com/questions/2787094/how-to-demonstrate-java-multithreading-visibility-problems – mmirwaldt May 23 '19 at 19:33

3 Answers3

4

What happens would really depend on how quickly doA() gets run:

If doA() changes the value of x before doB() reaches its synchronized block, then doB() will lock on the new object that doA() created.

If doB() is speedy and the synchronized(x) part gets evaluated before doA() can change the value of x, then doB() will have to wait until doA()'s synchronized block finishes.

Explanation

Whenever Java gets to the synchronized(x) code in either method, it evaluates the variable x, and that variable gives Java an object. So Java then tries to lock on that object, and stay locked on that object. Or, if there's already a lock on that object, it will wait for the lock to go away. (Whenever Java reaches the synchronized(x) code, it takes the value of the variable and ignores forgets the variable itself, so you can change the variable as you please afterwards, but locking and lock checking still happen on the variable's previous value.)

Kröw
  • 845
  • 1
  • 10
  • 26
  • 2
    And it's slightly more complicated than this because the reassignment of `x` may or may not be seen immediately by the thread calling `doB()`, depending on if it's `volatile`. – Slaw May 23 '19 at 18:59
  • so in the case of the first, since doA() does not have lock on the new object, doB() will not wait, but there is no collision of x in doA() and doB() because they are working off of different x – ealeon May 23 '19 at 19:07
  • @ealeon Yes. :) No waiting will occur since `doB()` will see an newly created, not-locked object when it starts running. – Kröw May 23 '19 at 19:07
  • 1
    Just to reiterate, unless `x` is `volatile`, the thread calling `doB()` **may** see a newly created, not-locked object in the first scenario. Without the `volatile` keyword, a thread is not guaranteed to see actions of another thread. So you could have one thread call `doB()` and still see the old value of `x`, another thread sees the most recent value, and a third thread sees the value as it was three reassignments ago. This reality of inter-thread communication is why synchronization/volatility exists in the first place—you need a _happens-before_ relationship. @ealeon – Slaw May 23 '19 at 19:13
  • @Slaw so your suggestion is to add the volatile keyword? – ealeon May 23 '19 at 19:21
  • 1
    @ealeon My suggestion, based on the given information, would be to not do this at all. How does it help you prevent CMEs? If you need to iterate a collection without locking it out completely, perhaps one of the collection classes in `java.util.concurrent` can help you; they have "weakly consistent" iterators and spliterators. But if you must do this, and you rely on `x` always being the most recent value, then yes, add the `volatile` keyword. – Slaw May 24 '19 at 05:09
2

My own curiosity got the best of me and I wrote this test to verify what happens. The lock is still held on the original reference and does not change.

import java.io.IOException;
import java.util.StringTokenizer;
import org.apache.hadoop.conf.Configuration;
import org.apache.hadoop.fs.Path;
import org.apache.hadoop.io.IntWritable;
import org.apache.hadoop.io.Text;
import org.apache.hadoop.mapreduce.Job;
import org.apache.hadoop.mapreduce.Mapper;
import org.apache.hadoop.mapreduce.Reducer;
import org.apache.hadoop.mapreduce.lib.input.FileInputFormat;
import org.apache.hadoop.mapreduce.lib.output.FileOutputFormat;

public class Test {

    private static Object x = new Object();

    public static void main(String[] args) throws Exception {
        Test a = new Test();
        Test b = new Test();

        Thread t = new Thread(a.new MyRunnable());
        t.start();
        Thread.sleep(100);
        b.doB();
    }

    public void doB() {
        System.out.println("here b");
        synchronized (x) {
            System.out.println("here sync b");
        }
        System.out.println("here done sync b");
    }

    public class MyRunnable implements Runnable {

        public MyRunnable() {
        }

        public void run() {
            System.out.println("here a");
            synchronized (x) {
                System.out.println("here sync a");
                try {
                    Thread.sleep(5000);
                } catch (InterruptedException e1) {
                    e1.printStackTrace();
                }

                x = new Object();

                System.out.println("here new object a");
                try {
                    Thread.sleep(10000);
                } catch (InterruptedException e) {
                    e.printStackTrace();
                }
                System.out.println("here after sleep a");
            }
            System.out.println("here done sync a");
        }
    }

}

Output is below. Clearly the lock is still held on to the original reference even after reassignment.

here a
here sync a
here b
here new object a
here after sleep a
here done sync a
here sync b
here done sync b
Mike Pone
  • 17,050
  • 12
  • 49
  • 63
  • 1
    It depends entirely on whether or not doB gets the new value of X first or the old one.. so even though your test works.. it only works sometimes.. Also depends on priority of threads.. Right now you are using one thread which will have a lower priority than the main thread.. If doB catches the value of the lock after doA has changed it, the code will be screwed.. – Brandon May 23 '19 at 19:28
  • If we make x as volatile then I guess output will change. When new object assignment is done the next thread reading x variable should see it's latest state and it could result in different outcome. – Rahul Vedpathak May 23 '19 at 19:43
1

I think the doA() method will still hold the lock on old "x". Other thread waiting for lock on "x" won't be notified unless the thread holding is finished processing synchronized block. Once synchronized block is completed, threads will fight to acquire lock on new instance of x.

Rahul Vedpathak
  • 1,106
  • 10
  • 23