How to properly kill a thread that is loading resources in the background

[Cornix]

Hi there,

in my application the user can load some files from disk. The loading is done in a separate thread spawned just for that purpose. At the same time a JDialog is created to show the progress of the loading.
This works great and all, but there is one problem, I want to give the user the possibility to close the JDialog and stop the loading of resources.

Now, I dont know how to properly kill the thread in such an event.
I will show some code:

		// Creates a new project. The project has not become the current project of the application yet.
		final Project project = new Project();
		project.setProjectPath(projectPath);
 
		// Creates the JDialog used to show the progress
		final DialogProgress progressDialog = new DialogProgress(this);
		progressDialog.setProject(project);
 
		// This thread will load the resources, dispose the dialog when all resources have been loaded, and make the project current in this application
		Thread loadingThread = new Thread() {
			public void run() {
				project.load();
				progressDialog.dispose();
				setProject(project);
			}
		};
		// Start thread and show dialog
		loadingThread.start();
		progressDialog.setVisible(true);

My plan was to add a WindowListener to my JDialog to find out when the window is closing.
Perhaps something like this:

		final Project project = new Project();
		project.setProjectPath(projectPath);
 
		final DialogProgress progressDialog = new DialogProgress(this);
		progressDialog.setProject(project);
 
		final Thread loadingThread = new Thread() {
			public void run() {
				project.load();
				progressDialog.dispose();
				setProject(project);
			}
		};
 
		// Makes sure the thread is stopped when the progress dialog was closed.
		progressDialog.addWindowListener(new WindowAdapter() {
			public void windowClosing(WindowEvent e) {
				loadingThread.stop();
				// The default close operation for the dialog is "DO_NOTHING_ON_CLOSE".
				progressDialog.dispose();
			}
		});
 
		loadingThread.start();
		progressDialog.setVisible(true);

But this is probably not a nice solution and I am afraid that files opened by the project might not be closed correctly.

Any suggestions? I really appreciate the help.

[ChristopherLowe]

I'm not really a swing programmer but something I use a lot in Android for this sort of thing is the callback design pattern. Make your DialogProgress an abstract class with an abstract method onUserCancel. Call this method when the user hit's the cancel button or whatever but leave the implementation of that method to be handled where it is created.

DialogProgress progressDialog = new DialogProgress(this) {
 
    public void onUserCancel() {
         //close the file streams, threads etc
    }
 
});

The rationale is that the your dialog doesn't know how it's supposed to handle the close operation and so it shouldn't be responsible for it. This separation of concerns is nice but it's the reusability that I like because next time you use the class you will get a syntax error to remind you to implement that method.

[Cornix]

I'm not really a swing programmer but something I use a lot in Android for this sort of thing is the callback design pattern. Make your DialogProgress an abstract class with an abstract method onUserCancel. Call this method when the user hit's the cancel button or whatever but leave the implementation of that method to be handled where it is created.

DialogProgress progressDialog = new DialogProgress(this) {
 
    public void onUserCancel() {
         //close the file streams, threads etc
    }
 
});

The rationale is that the your dialog doesn't know how it's supposed to handle the close operation and so it shouldn't be responsible for it. This separation of concerns is nice but it's the reusability that I like because next time you use the class you will get a syntax error to remind you to implement that method.

In swing this is already done by the WindowListener that I added to the JDialog. It has the exact same purpose.
My question is more about how to properly kill a running thread.
Usually one is not supposed to kill a thread, instead you should have it terminate after it finished all of its code.
But my thread might be waiting for IO operations, I want to kill it without having to wait for the IO operations to finish.

[ChristopherLowe]

True, Android has an analogy with the onDismissListener. My point is though that it's better design to have the dialog itself define this listener and the caller respond to it. This way it doesn't violate MVC.

Anyway to address the question, a volatile flag can be used if this is a blocking operation.

Thread loadingThread = new Runnable() {
 
    private volatile boolean isRunning = true;
 
    public void terminate() {
         isRunning = false;
    } 
 
    public void run() {
          while (isRunning) {
                // stuff
          }
    }
}

If it's not a blocking operation you could just use Thread.interrupt()

[Cornix]

But what would that while-loop help?
I posted the code, the thread will simply initiate the loading of resources and then terminate by itself.
My problem has to do with opened files that need to be properly closed when the user cancels the dialog.

[ChristopherLowe]

Ahh, this is why I brought up MVC and callbacks. Your problem isn't technical it's design. This line here:

project.load();

is code smell. Without seeing the code I can assume that it opens a file stream and deserializes the project. In essence the model (project) is also the controller (serializing and deserializing). While a .load() method may make sense to begin with it starts to fall over when you need actual control, like stopping the stream. Point in fact; would a ByteInputStream make sense as a member variable of Project? Nope, but it what would make sense if the serialize/deserialize was in it's own class and acted as a controller.

 
public abstract class ProjectLoader {
 
    private ByteInputStream _bis;
    private Project _project;
 
    public ProjectLoader() { }
 
    public Project loadProject()  {
         _bis = new ByteInputStream() //etc...
         //populate the Project class
         _project = new Project();
         onComplete();
    }
 
    public Project getProject() {
         return _project;
    }
 
   public void stopLoading() {
        _bis.close();
       onUserCancelled();
   }
 
   public abstract void onComplete();
   public abstract void onUserCancelled();
 
}

This is pretty crude but demonstrates what I mean. It gives you a safe way of handling the input streams and callbacks for completion and cancellation that the views can act on.

[Cornix]

The project is a model with a tree structure of separate resources.
Resources in the project are saved as separate files over a wide range of directories. How many, and where, is not known to the GUI since it has nothing to do with it.
The project knows how it is managed and where the files are and it initializes the loading, and then the evaluation of all files in the correct sequence.

I want to keep the loading logic separate from the GUI because it might change at some point in time.
I am not quite sure what I should do. Maybe I should do the loading in separate steps, loading one file after the other. Then I should outsource the loading logic into a separate class. But the question remains, how do I kill a thread that is waiting on an IO operation?

[ChristopherLowe]

Not much more I can add I'm afraid. Just .interrupt() the thread, wrap up .start() in an InterruptedExpection and cross your fingers that the IO streams respect the interrupted thread state. java.nio handles threading a lot better that java.io but there is still this risk you will get a deadlock the next time round due to streams that aren't closed.

Another option may be to use a FutureTask which is "A cancellable asynchronous computation". Again, it's hard to know how the streams will behave without giving it a try.

Good luck, let me know how it goes.

[DuncanS]

As outlined here, ChristopherLowe's suggestions are your best bet for killing the thread abruptly, but I would wrap it in a ShutdownHook -

Runtime.getRuntime().addShutdownHook(new Thread() {
  @Override
  public void run() {
    try {
      // Kill your thread
    } catch (Exception e) {
      // Log states when you crash the thread
    } finally {
      // Close IO streams
    }
  }
});

I prefer this to a WindowListener because it will run no matter how the application is closed. If your IO operations don't play nice with .interrupt(), it's probably best to let them finish.

[Cornix]

It happens when a JDialog is closed, not when the entire program terminates. So I dont think a shutdown hook would work.

[DuncanS]

If that's the case, shouldn't a graceful shutdown be fine? Something like this:

public class MyThread implements Runnable {
 
  private volatile Thread myThread;
 
  public MyThread() {
    myThread = new Thread(this);
    myThread.start();
  }
 
  @Override
  public void run() {
    Thread running = Thread.currentThread();
    while (myThread == running) {
      try {
        // IO operations
      } catch (Exception e) {
      }
    }
  }
 
  public void terminate() {
    myThread.interrupt();
    try {
      // Close your streams
      myThread = null;
    } catch (Exception e) {
    }
  }
 
}