Synchronized Block inside run method

Question

If I have something like below, so what does that mean inside synchronized block

synchronised (syncObject) {

Basically, it will means only one thread can be inside the above block and as soon as one thread is finished executing, second thread will enter that synchronized block synchronised (syncObject). Right? Can anyone explain to me in a LayMan language so that I can get better picture?

private static final class Task implements Runnable {
{
  private static Object syncObject = new Object();

    public Task(Command command, BlockingQueue<Integer> pool1, BlockingQueue<Integer> pool2) {
    this.command = command;
    this.existPool = pool1;
    this.newPool = pool2;
}

  public void run()
  {
    synchronised (syncObject) {
      if() {
        existId = existPool.take();
        attributeMethod(existId);
        } else if() {
            newId = newPool.take();
            attributeMethod(newId);
        }
    }
  }
}

// So I need to make this method synchronized or not? Currently I have made this synchronized
private synchronized void attributeMethod(int range) {
    // And suppose If I am calling any other method here-

 sampleMethod();
}


// What about this method, I need to make this synchronized as well? or not?
private synchronized void sampleMethod() {


}

Answer 1

Basically, it will means only one thread can be inside the above block and as soon as one thread is finished executing, second thread will enter that synchronized block synchronised (syncObject). Right?

Right!

So I need to make this method synchronized or not?

No you don't. Assuming that the method is only going to be called from within the synchronized block in the run() method, that block will already prevent multiple threads from executing the method simultaneously. So declaring the method to be synchronized is redundant.

However, I should point out some things:

• When you declare an instance method as synchronized, it will synchronize on this; i.e. on the Task object. But your synchronized block is synchronizing on a different object ... the object in syncObject. In this case, this doesn't matter. However, if the synchronized block in the run() method wasn't there, you would find that the threads were attempting synchronizing on different objects ... and you would NOT get mutual exclusion.

• By synchronizing at the top level of the run() method ... using a single shared syncObject for all threads that execute that task ... you are effectively making the tasks run one at a time. This completely negates any benefits of using threads.

• It is good practice to declare the variable containing a private lock object (such as syncObject) to be final. This avoids the possibility that something might overwrite it ... resulting in a synchronization failure.

Answer 2

No, attributeMethod is already running within the scope of a synchronized block; no need to mark it as such, unless you intend to call it concurrently outside this block.