Search code examples
javamultithreadingparallel-processingjava-threadsthread-synchronization

Programme not Terminating in Multi-threaded environment -Java


Trying to make a simple multi-threaded programme where it prints Factorial series where each number is printed by different Thread and at the end I am giving a report of which number printed by which thread.I have got the desired output but somehow my program is not terminating.

Constraint: I am not allowed to use Concurrent Package

import java.util.ArrayList;
import java.util.Scanner;

class Report {

  private long factorial;
  private String threadName;
  private int activeThreads;

  public Report(long factorial, String threadName, int activeThreads) {
      this.factorial = factorial;
      this.threadName = threadName;
      this.activeThreads = activeThreads;
  }

  public long getFactorial() {
      return factorial;
  }

  public String getThreadName() {
      return threadName;
  }

  public int getActiveThreads() {
      return activeThreads;
  }

  public void setActiveThreads(int activeThreads) {
      this.activeThreads = activeThreads;
  }

}

public class Factorial implements Runnable {

  public static ArrayList<Report> report = new ArrayList<Report>();
  private static int count;

  public static void main(String[] args) throws InterruptedException {

      Scanner in = new Scanner(System.in);

      System.out.print("N: ");
      int n = in.nextInt();
    
      count = n;
    
      Factorial f = new Factorial();
      f.series(n);
    
      Thread.sleep(1000);
    
      // Series
      for(Report r : report) {
          if(r.getFactorial() == 1) {
              System.out.print(r.getFactorial());
          }
          else {
              System.out.print(r.getFactorial() + "*");
          }
      }
    
      System.out.println();
    
      // Report
      for(Report r : report) {
          System.out.println(r.getFactorial() + " printed by " + r.getThreadName() + " " + r.getActiveThreads());
      }
      ThreadGroup threadGroup = Thread.currentThread().getThreadGroup();
      System.out.println("In Main");
    
      in.close();
  }

  public void series(int n) throws InterruptedException {
      for(int i=0;i<n;i++) {
          Thread t = new Thread(new Factorial());
          t.start();
      }
  }

  public synchronized void generate() {
      ThreadGroup threadGroup = Thread.currentThread().getThreadGroup();
      report.add(new Report(count--, Thread.currentThread().getName(), threadGroup.activeCount()));
      notifyAll();
      System.out.println("In generate" + threadGroup.activeCount());
  }
    


  @Override
  public void run() {
      generate();
      synchronized (this) {
          try {
              wait();
          }
          catch(Exception e) {
              e.printStackTrace();
          }
      }
      ThreadGroup threadGroup = Thread.currentThread().getThreadGroup();
      System.out.println("In Run" + threadGroup.activeCount());
  }

  public static int getCount() {
      return count;
  }

  public static void setCount(int count) {
      Factorial.count = count;
  }

}

Although I know that we can kill the threads using .stop() but I think it's not recommended.


Solution

  • To make synchronization effective (synchronized, wait, notify), you have to use the same instance.
    In series, you create a new Factorial instance on each loop, making every thread to wait indefinitely.

    public void series(int n) throws InterruptedException {
      for(int i=0;i<n;i++) {
          // Thread t = new Thread(new Factorial()); // creates an new instance
          Thread t = new Thread(this);
          t.start();
      }
    }
    

    In the run method, you first call notifyAll() (through generate), and then wait.
    The last created thread will wait after all the others are done.
    One way or another, this last thread has to be notified. It could be right after the sleep call, with:

    synchronized(f) {
        f.notify();
    }
    

    or maybe with a dedicated synchronized method.