What is the correct way to avoid an empty synchronized block?

78 views Asked by At

Recently I've started looking into multithreading, and I have a question, perhaps more experienced ones could help. My program creates two parallel threads, each of them prints counts from 0 to 19 (the NumbersPrinter class, which implements the Runnable interface).

class NumbersPrinter implements Runnable {
  private Mediator mediator;
  private String name;
  private int makeActionOnCount;
  
  public NumbersPrinter(Mediator mediator, String name, int makeActionOnCount) {
    this.mediator = mediator;
    this.name = name;
    this.makeActionOnCount = makeActionOnCount;
  }   
  
  
  @Override
  public void run() {
    
    for(int i = 0; i<20; i++){
      
      try {
        synchronized(this.mediator) {
          if(this.mediator.actionInProgress.get()) {
           System.out.println(name + " waits");
           wait();
          }
        }
        System.out.println(this.name + " says " + i);     
        Thread.sleep(500);
        if(i == makeActionOnCount) {
          synchronized(this.mediator) {
            System.out.println(this.name + " asks Mediator to perform action...");
            this.mediator.performAction();
            this.mediator.notify();
          }
        }
        
      } catch (InterruptedException e) {
        e.printStackTrace();
      }
    }
    
    
  }
}

When one of the threads reaches a certain number (defined in the makeActionOnCount variable), it starts performing a certain action that stops the execution of the second counter. The action lasts 5 seconds and after that both counters continue to count. The counters are interconnected through an instance of the Mediator class, the performAcyion() method also belongs to the instance of the Mediator class.

import java.util.concurrent.atomic.AtomicBoolean;

class Mediator {
  public AtomicBoolean actionInProgress = new AtomicBoolean(false);
  
  public Mediator() {
    
  }
  
  public void performAction() throws InterruptedException {
    
    actionInProgress.set(true); 
    System.out.println("Action is being performed");
    Thread.sleep(5000);   
    System.out.println("Action has been performed");
    actionInProgress.set(false);        
    
  }
}

Here's the Main class:

class Main {
  
 
  public static void main(String[] args) throws InterruptedException{   
    
    
    Mediator mediator = new Mediator();
    
    NumbersPrinter data = new NumbersPrinter(mediator, "Data", 10);
    NumbersPrinter lore = new NumbersPrinter(mediator, "Lore", 5);
    
    Thread oneThread = new Thread(data);
    Thread twoThread = new Thread(lore);    
    
    System.out.println("Program started");
    
    oneThread.start();
    twoThread.start();
    oneThread.join();
    twoThread.join();
    
    System.out.println("Program ended");
  }

The way the program is written now - works fine, but I don't quite understand what exactly should I write in the first synchronized block, because if you delete all content from it, the program still works, since the counter that does not execute the performAction() method stops 'cause the counter cannot access the monitor of the Mediator object 'cause it is busy with the parallel counter. AtomicBoolean variable and checking it also makes no sense. In other words, I may not use the wait () and notify () constructs at all, as well as the value of the AtomicBoolean variable, and just check access to the Mediator object's monitor every new iteration using an empty synchronized block. But I've heard that an empty synchronized block is a bad practice.

I am asking for help on how to rewrite the program to use the synchronized block and the wait() and notify() methods correctly. Maybe I'm syncing on the wrong object? How would you solve a similar problem? Thanks in advance

0

There are 0 answers