What's Wrong with This Code? Java Swing and Threading

By Marcus Zarra  |  Posted 2005-07-09 Email Print this article Print
 
 
 
 
 
 
 

WEBINAR: Event Date: Tues, December 5, 2017 at 1:00 p.m. ET/10:00 a.m. PT

How Real-World Numbers Make the Case for SSDs in the Data Center REGISTER >

Ready to test your coding knowledge? Marcus Zarra challenges you to examine two Java Swing examples, specifically using threads; each are wrong in a subtle but painful way. See if you can identify the problem — and then learn the best way to correct

Ready to test your coding knowledge?

In this article, I challenge you to examine two Java Swing examples, specifically using threads. Each are wrong, in a subtle but painful way.

See if you can identify the problem before I show you the best way to correct the offending code.

Let's start with a thread blocking example.

I created a simple Swing class file. I am sure this is extremely common. On the surface, it looks perfectly acceptable. In this class, I extend from a JFrame and add a simple button to the frame.

Take a look at the code, and see if you can spot the issues:


 

package com.zarrastudios.example;

import javax.swing.*;
import java.awt.*;
import java.awt.event.ActionListener;
import java.awt.event.ActionEvent;
import java.awt.event.WindowAdapter;
import java.awt.event.WindowEvent;

public class FrameExample1 extends JFrame {
  private JButton btnActivate;

  public FrameExample1() {
    super("Frame Example 1");
    initAndLayoutComponents();
    initListeners();
    pack();
  }

  private void initListeners() {
    btnActivate.addActionListener(new ActionListener() {
      public void actionPerformed(ActionEvent event) {
        //Simulate database access
        try {
          Thread.sleep(100000);
        } catch (InterruptedException e) {
          e.printStackTrace();
        }
      }
    });
  }

  private void initAndLayoutComponents() {
    btnActivate = new JButton("Activate");

    JPanel main = new JPanel(new BorderLayout());
    JPanel north = new JPanel(new FlowLayout());
    JPanel center = new JPanel();

    center.setBackground(Color.pink);

    north.add(btnActivate);

    main.add(north, BorderLayout.NORTH);
    main.add(center, BorderLayout.CENTER);

    setContentPane(main);
  }

  public static void main(String args[]) {
    FrameExample1 fe1 = new FrameExample1();
    fe1.addWindowListener(new WindowAdapter() {
      public void windowClosing(WindowEvent event) {
        System.exit(0);
      }
    });
    fe1.setVisible(true);
  }
}

I'll give you the answers on the next page. But before you move forward, do your best to identify the problem.

Thread Blocking: The Answers

This is a relatively simple problem, but it is unbelievably common. The answer is: There is non-GUI work occurring in the event thread.

What does this mean? The long answer is, naturally, more complicated. Swing is a single threaded model. Everything that occurs in relation to the GUI happens in a single thread. When you click a button, that event is fired in the event thread. This same thread is responsible for drawing the GUI. Therefore, in the example above, my button action causes a very large problem.

To test this problem, run the application as it is shown. Once the GUI is displayed, resize it a bit, so that the center square is larger and easier to see. Next, hit the activate button. This button is designed to have a very large delay — simulating database activity, or some other lengthy back end process. Once you push the button, go ahead and resize the window again.

Notice anything different? The center no longer resizes to match the window size! In fact, you can't do anything with this window that requires the window's code to respond. Depending on your operating system, you might be able to keep the window redrawing correctly, but you cannot close the window, nor can you get any of the GUI components to resize properly.

Because the simulated database activity is occurring inside of the event thread, that thread is busy, and cannot process the other events that are flowing into it. When you resize the window, an event is sent — but that event is sitting in the stack waiting for the database activity to complete before it can be processed. The same goes for a window closing event, and every other GUI event that is not explicitly handled by the operating system.

Naturally, this is a bad design. The way to solve this is actually very simple and takes just a small amount of extra code.

The correct way to handle this is to execute the simulated database code inside of a separate thread. Thankfully, threading is very simple in Java. Below, I have reworked the action listener to spin off a separate thread to handle the database code.

  private void initListeners() {
    btnActivate.addActionListener(new ActionListener() {
      public void actionPerformed(ActionEvent event) {
        DatabaseSimulator ds = new DatabaseSimulator();
        ds.start();
      }
    });
  }

To make this method as clean and maintainable as possible, I moved the database handling code into its own separate inner class, as follows:

  class DatabaseSimulator extends Thread {
    public void run() {
      //Simulate database access
      System.out.println("Starting simulated database access");
      try {
        Thread.sleep(100000);
      } catch (InterruptedException e) {
        e.printStackTrace();
      }
      System.out.println("Simulated database access is complete");
    }
  }

Once these changes are implemented, the test class runs remarkably smoother. Now, when the button is pressed, the GUI does not delay at all. Even while the database code is running in the background, the GUI is fully responsive and can receive input from the user.

This code change introduced a second error, however. If you click on the activate button more than once, multiple database threads are spawned and start running. One way to prevent this from happening is to disable the activate button after it has been pressed, and reactivate it after the database code is complete. In the next section, I will detail the safest way to do this.

Race Condition: The Code

For the second example, I created another class that extends JFrame.

This time, however, I put in two JButtons and a JProgressBar. One of the buttons starts a background timer that updates the progress bar, and the second button stops the background timer. The error in this code is quite a bit more subtle than the first.

See if you can spot the issues with it. Again: do your best to identify the problem before you peek at the answer!

package com.zarrastudios.example;

import javax.swing.*;
import java.awt.event.WindowAdapter;
import java.awt.event.WindowEvent;
import java.awt.event.ActionListener;
import java.awt.event.ActionEvent;
import java.awt.*;

public class FrameExample2 extends JFrame {
  private JButton activate, shutdown;
  private JProgressBar progressBar;
  private Ticker ticker;

  public FrameExample2() {
    super("Frame Example 2");
    initAndLayoutComponents();
    initListeners();
    pack();
  }

  private void initListeners() {
    activate.addActionListener(new ActionListener() {
      public void actionPerformed(ActionEvent evt) {
        ticker = new Ticker();
        ticker.start();
        activate.setEnabled(false);
        shutdown.setEnabled(true);
      }
    });
    shutdown.addActionListener(new ActionListener() {
      public void actionPerformed(ActionEvent evt) {
        ticker.shutdown();
        activate.setEnabled(true);
        shutdown.setEnabled(false);
      }
    });
    addWindowListener(new WindowAdapter() {
      public void windowClosing(WindowEvent event) {
        System.exit(0);
      }
    });
  }

  private void initAndLayoutComponents() {
    JPanel main = new JPanel(new BorderLayout());
    JPanel north = new JPanel(new FlowLayout());
    JPanel south = new JPanel(new FlowLayout());
    activate = new JButton("Activate");
    shutdown = new JButton("Shutdown");
    shutdown.setEnabled(false);
    progressBar = new JProgressBar();

    north.add(activate);
    north.add(shutdown);

    south.add(progressBar);

    main.add(north, BorderLayout.NORTH);
    main.add(south, BorderLayout.SOUTH);

    setContentPane(main);
  }

  public static void main(String args[]) {
    FrameExample2 fe2 = new FrameExample2();
    fe2.addWindowListener(new WindowAdapter() {
      public void windowClosing(WindowEvent event) {
        System.exit(0);
      }
    });
    fe2.setVisible(true);
  }

  class Ticker extends Thread {
    private boolean running = true;

    public void shutdown() {
      running = false;
      interrupt();
    }

    public void run() {
      System.out.println("Starting counter");
      for (int counter = 1; counter <= 10; counter++) {
        if (!running) break;
        progressBar.setValue(counter * 10);
        try {
          Thread.sleep(1000);
        } catch (Exception e) {
          //Expected exception
          System.out.println("Sleep interrupted");
        }
      }
      System.out.println("Counter is finished");
    }
  }
}

Race Condition: The Answers

This example is quite a bit more involved than the first one. Even though it is more complex, the problem in this code is harder to see, and even harder to reproduce in such a simple example. The answer is: A GUI element is being modified outside of the event thread.

But wait. When the example is run, nothing unusual happens. Everything runs as it should. What is the problem?

As discussed earlier, Swing is single threaded. It is also not thread safe. Therefore, altering a GUI element outside of the event thread breaks the single thread model, and puts the GUI at risk.

The problem with this error is that it does not show up when the example is small, and it does not show up consistently. Therefore, even though this example does not make the problem clear, it is there. There is a race condition in this code. It is quite possible that the GUI is in the process of updating itself while a value gets changed outside of the event thread. This causes the GUI to paint itself incorrectly and inconsistently.

To correct this error, all GUI code needs to be executed within the event thread. There are a couple of ways to handle this. The first way is to utilize the javax.swing.SwingUtilities. Inside those utilities are two methods, invokeLater() and invokeAndWait(). Both of those methods accept a java.lang.Runnable object as a parameter. The only difference between them is that invokeAndWait() blocks the current thread until the Runnable is processed; invokeLater() does not block. Using one of these methods in the example code would change the Ticker's run() to look like this:

    private int counter;

    public void run() {
      System.out.println("Starting counter");
      for (counter = 1; counter <= 10; counter++) {
        if (!running) break;
        Runnable r = new Runnable() {
          public void run() {
            progressBar.setValue(counter * 10);
          }
        };
        SwingUtilities.invokeLater(r);
        try {
          Thread.sleep(1000);
        } catch (Exception e) {
          //Expected exception
          System.out.println("Sleep interrupted");
        }
      }
      System.out.println("Counter is finished");
    }

Unfortunately, in this example, utilizing these methods causes two problems.

  1. I am creating and destroying an object on each cycle, which is wasteful.
  2. I have to change the counter to a class variable, so that the Runnable object can access it. That is a bit messy.

Another way to get the JProgressBar update into the event thread where it belongs is to use a javax.swing.Timer object. A Timer object accepts a delay and an ActionListener. The timer then fires the ActionListener based on tthat delay. The code contained within the ActionListener is executed on the event thread, so the single thread model is preserved.

Utilizing the Timer solution changes the example code to look like this:

package com.zarrastudios.example;

import javax.swing.*;
import java.awt.event.WindowAdapter;
import java.awt.event.WindowEvent;
import java.awt.event.ActionListener;
import java.awt.event.ActionEvent;
import java.awt.*;

public class FrameExample2 extends JFrame {
  private JButton activate, shutdown;
  private JProgressBar progressBar;
  private Timer timer;

  public FrameExample2() {
    super("Frame Example 2");
    initAndLayoutComponents();
    initListeners();
    pack();
  }

  private void initListeners() {
    activate.addActionListener(new ActionListener() {
      public void actionPerformed(ActionEvent evt) {
        startTimer();
        activate.setEnabled(false);
        shutdown.setEnabled(true);
      }
    });
    shutdown.addActionListener(new ActionListener() {
      public void actionPerformed(ActionEvent evt) {
        timer.stop();
        activate.setEnabled(true);
        shutdown.setEnabled(false);
      }
    });
    addWindowListener(new WindowAdapter() {
      public void windowClosing(WindowEvent event) {
        System.exit(0);
      }
    });
  }

  private void startTimer() {
    progressBar.setValue(0);
    ActionListener al = new ActionListener() {
      public void actionPerformed(ActionEvent evt) {
        int value = progressBar.getValue();
        value += 10;
        progressBar.setValue(value);
      }
    };
    timer = new Timer(1000, al);
    timer.start();
  }

  private void initAndLayoutComponents() {
    JPanel main = new JPanel(new BorderLayout());
    JPanel north = new JPanel(new FlowLayout());
    JPanel south = new JPanel(new FlowLayout());
    activate = new JButton("Activate");
    shutdown = new JButton("Shutdown");
    shutdown.setEnabled(false);
    progressBar = new JProgressBar();

    north.add(activate);
    north.add(shutdown);

    south.add(progressBar);

    main.add(north, BorderLayout.NORTH);
    main.add(south, BorderLayout.SOUTH);

    setContentPane(main);
  }

  public static void main(String args[]) {
    FrameExample2 fe2 = new FrameExample2();
    fe2.addWindowListener(new WindowAdapter() {
      public void windowClosing(WindowEvent event) {
        System.exit(0);
      }
    });
    fe2.setVisible(true);
  }
}

An inner class is no longer needed, since the Timer object handles all of the delay. Since the ActionListener's code is already executing on the event thread, I do not need either the invokeLater() or invokeAndWait() methods from the SwingUtilties... and there is no race condition!

These two examples are, easily, the most common mistakes made when writing Java Swing code. Either of these two coding mistakes can cause the GUI to run in an unpredictable manner and give a poor experience to the user. Combined, as they often are, these flaws create a GUI that is extremely unstable and unpredictable.

 
 
 
 
 
 
 
 
 
























 
 
 
 
 
 

Submit a Comment

Loading Comments...
























 
 
 
 
 
 
 
 
 
Thanks for your registration, follow us on our social networks to keep up-to-date