When should I use a synchronized method and when a synchronized block in Java?

Question

I'm studying how to synchronize methods and blocks in Java in order to avoid race condition and I tried to solve an exercise in both ways. The problem is that if I try to use synchronized block everything works fine but with synchronized method it get stuck. I thought that I could use both way with not really big differences(maybe one of them reduce parallelism in some case but I'm not sure about this). I'm wondering what's wrong in my code and I want to ask If there's any case where It's preferible using synchronized block instead of synchronized method.

//Not working

import java.util.Random;

class MultiplicationTable extends Thread {
    private Cont obj;
    private int number;
    private Random r;

    public MultiplicationTable(Cont o, int num) {
        obj = o;
        number = num;
        r = new Random();
        start();
    }

    public void run() {

        for (int j = 0; j < 10; j++) {
            for (int i = 0; i < number; i++) {
                obj.incr();
                try {
                    Thread.sleep(100);
                } catch (InterruptedException e) {
                }
            }
            System.out.println(Thread.currentThread().getName() + ": " + obj.getVal());
        }
        try {
            Thread.sleep(r.nextInt(2000));
        } catch (InterruptedException e) {
        }
    }
}

class Cont {
    private int count = 0;
    private boolean available = false;

    public synchronized void incr() {
        while (available) {
            try {
                wait();
            } catch (InterruptedException e) {
                // TODO: handle exception
            }
        }
        available = true;
        count++;
        notifyAll();
    }

    public synchronized int getVal() {
        while (!available) {
            try {
                wait();
            } catch (Exception e) {
                // TODO: handle exception
            }
        }
        available = false;
        notifyAll();
        return count;
    }
}

public class Es3 {
    public static void main(String[] args) {
        Cont obj = new Cont();
        int num = 5;
        MultiplicationTable t1 = new MultiplicationTable(obj, num);
        MultiplicationTable t2 = new MultiplicationTable(obj, num);
    }
}

//Working

 import java.util.Random;

class MultiplicationTable extends Thread {
    private Cont obj;
    private int number;
    private Random r;

    public MultiplicationTable(Cont o, int num) {
        obj = o;
        number = num;
        r = new Random();
        start();
    }

    public void run() {
        synchronized (obj) {
            for (int j = 0; j < 10; j++) { 
                for (int i = 0; i < number; i++) {
                    obj.incr();
                    try {
                        Thread.sleep(100);
                    } catch (InterruptedException e) {
                        System.out.println(e.getMessage());
                    }
                }
                System.out.println(Thread.currentThread().getName() + ": " + obj.getVal());
            }
            try {
                Thread.sleep(r.nextInt(2000));
            } catch (InterruptedException e) {
                System.out.println(e.getMessage());
            }
        }
    }
}

class Cont {
    private int count = 0;

    public void incr() {
        count++;
    }

    public int getVal() {
        return count;
    }
}

public class Es3 {
    public static void main(String[] args) {
        Cont obj = new Cont();
        int num = 5;
        MultiplicationTable t1 = new MultiplicationTable(obj, num);
        MultiplicationTable t2 = new MultiplicationTable(obj, num);
    }
}

Answer 1

I don't think this is a dupe because, despite the title, the actual problem is the OP's specific implementation. There's a bug in the code, it's not a question of methods vs. blocks.

The bug in your code is where you try to implement a locking mechanism. In incr(), you wait until available is set to false, which only happens in getVal():

public synchronized void incr() {
    while (available) { // <-- bug
        try {
            wait();

Since your loop only calls incr() without calling getVal(), both threads become stuck after the first call to incr(). (You call getVal() eventually, but only after the inner loop is complete. Both threads are good and stuck by then.)

Solution: AtomicInteger doesn't have weird bugs like this. If you're trying to implement some kind of producer/consumer mechanism, then one of the concurrent queues (like ArrayBlockingQueue) is a better solution.

Answer 2

1. One significant difference between synchronized method and block is that, Synchronized block generally reduce scope of lock. As scope of lock is inversely proportional to performance, its always better to lock only critical section of code. One of the best example of using synchronized block is double checked locking in Singleton pattern where instead of locking whole getInstance() method we only lock critical section of code which is used to create Singleton instance. This improves performance drastically because locking is only required one or two times.

2. Synchronized block provide granular control over lock, as you can use arbitrary any lock to provide mutual exclusion to critical section code. On the other hand synchronized method always lock either on current object represented by this keyword or class level lock, if its static synchronized method.

3. Synchronized block can throw throw java.lang.NullPointerException if expression provided to block as parameter evaluates to null, which is not the case with synchronized methods.

4. In case of synchronized method, lock is acquired by thread when it enter method and released when it leaves method, either normally or by throwing Exception. On the other hand in case of synchronized block, thread acquires lock when they enter synchronized block and release when they leave synchronized block.