Friday, January 9, 2009

On Never Using System.exit()

In one of the code reviews, I got a feedback to not use System.exit() function call at all. This was a little surprising for me but the explanation for that rule was very simple and made total sense. It's because there may be some worker thread is doing some important job (like committing a database transaction) and System.exit() will abruptly kill those threads leading to nasty bugs as you already might know.

The scenario is roughly as follows:

// A program that uses System.exit().

// MainUsingSystemExit.java
import java.io.BufferedReader;
import java.io.IOException;
import java.io.InputStreamReader;

public class MainUsingSystemExit {
  public static void main(String[] args)
      throws IOException { // You should never do this, I'm doing here for the
                           // sake of brevity of the example in the blog.
    DatabaseThread dbThread = new DatabaseThread();
    dbThread.start();

    BufferedReader reader
        = new BufferedReader(new InputStreamReader(System.in));
    while (true) {
      // TIP: In the code below, the letter 'N' is uppercase, you know why? It's
      // because that's the default answer if you don't enter anything. This is
      // a very common convention in CLI apps.
      System.out.print("Do you want to terminate? [y/N]: ");
      String userInput = reader.readLine();

      if ("y".equalsIgnoreCase(userInput)) {
        // The problem with the System.exit() below is simple, the
        // DatabaseThread is killed and that's very bad as you know.
        System.exit(0);
      }
    }
  }
}

// DatabaseThread.java
public class DatabaseThread extends Thread {
  public void run() {
    // Some db related operation.
    while (true) {
      try {
        Thread.sleep(5000);
      } catch (InterruptedException e) {}

      System.out.println("Database thread is still alive.");
    }
  }
}

So, how do we deal with such a code, in a clean way from the design point of view too? Solution: Observer pattern to the rescue!

We'll create a listener interface called ShutdownListener which will have a single method called shutdown() which is invoked when the system is about to go down. Those who'd like to clean-up stuff before shutdown can implement the interface and do their clean-up in the shutdown() method.

// A program that DOES NOT use System.exit().
public interface ShutdownListener {
  // Called when it's time to shutdown.
  public void shutdown();
}

And the DatabaseThread is one such class that should implement the ShutdownListener interface.

// DatabaseThread.java (modified)
public class DatabaseThread
    extends Thread
    implements ShutdownListener {
  private boolean shutdown;

  public void run() {
    // Some db related operation.
    while (!shutdown) {
      try {
        Thread.sleep(5000);
      } catch (InterruptedException e) {}

      System.out.println("Database thread is still alive.");
    }

    System.out.println("Database thread going down.");
  }

  public void shutdown() {
    shutdown = true;
    this.interrupt();
  }
}

The synchronization issues aren't handled here to not clutter the code in the blog but should be taken care.

We'll have a ShutdownHandler class where all the listeners will be registered. It can also register a shutdown hook* which does nothing but calls its own initiateShutdown() method which goes and calls the shutdown() method of all the registered ShutdownListeners.

import java.util.ArrayList;
import java.util.Iterator;
import java.util.List;

public class ShutdownHandler {
  private List shutdownListeners;

  public ShutdownHandler() {
    shutdownListeners = new ArrayList();

    // For handling TERM, INT signals.
    Runtime.getRuntime().addShutdownHook(new Thread() {
      public void run() {
        ShutdownHandler.this.initiateShutdown();
      }
    });
  }

  public void registerShutdownListener(ShutdownListener listener) {
    shutdownListeners.add(listener);
  }

  public void initiateShutdown() {
    Iterator iterator = shutdownListeners.iterator();

    while (iterator.hasNext()) {
      ShutdownListener listener = (ShutdownListener) iterator.next();
      listener.shutdown();
    }
  }
}

And finally, the new main():

import java.io.BufferedReader;
import java.io.IOException;
import java.io.InputStreamReader;

public class MainWithoutSystemExit {
  public static void main(String[] args) throws IOException {
    DatabaseThread dbThread = new DatabaseThread();
    dbThread.start();

    ShutdownHandler shutdownHandler = new ShutdownHandler();
    shutdownHandler.registerShutdownListener(dbThread);

    BufferedReader reader
        = new BufferedReader(new InputStreamReader(System.in));
    while (true) {
      System.out.print("Do you want to terminate? [y/N]: ");
      String userInput = reader.readLine();

      if ("y".equalsIgnoreCase(userInput)) {
        // Calling the ShutdownHandler's shutdown() method
        // instead of System.exit()
        shutdownHandler.initiateShutdown();
        return;
      }
    }
  }
}

As you can see, the ShutdownHandler's initiateShutdown() is called instead of System.exit() in the new main() method. Now if we've a new SocketThread or some other thread that needs a graceful exit, all we've to do is implement ShutdownListener and register it with the ShutdownHandler.

Now, go look in your code and see how you can eliminate System.exit() calls altogether. See if that brings out a better design. Or do you think this idea of not using System.exit() is just crazy?

* Thanks to Ajeya for reminding me about shutdown hooks.

1 comments:

Federico said...

Very useful! thanks for posting it!

Post a Comment