Ensuring threads running infinite loops are terminated before exiting the program

Question

As a hobbyist programmer, I gain knowledge where I can find it from google searches. I tend to write code that utilizes threads that have infinite loops using while loops, where the while condition slaves onto a BooleanProperty that when set false, causes the thread to interrupt the loop. I use a single class to keep track of the threads that are still looping and when they have all stopped, it can then exit the program.

A specific example of how I use a thread like this is an app I wrote that keeps my One Time Passwords for two-factor authentication on various web sites. Those passwords change every 30 seconds at second 0 of a minute change and at second 30 within a minute. Within each 30-second time frame, I do things to the label that is showing the password for an account such as fade the color to red during the last 10 seconds, then fade opacity in the last second and the first second, and there is simply no possible way to make those changes event-driven, so I use a looping thread to determine when those events need to happen.

However, it was brought to my attention that the way I am doing this might not be the best way, so I would like to share how I am doing this and see if anyone can suggest a better method.

I took a few Java classes when I got my CIS degree, but nothing covering advanced topics in Java. So I admit my knowledge is far from where I want it to be where wielding the Java language is concerned. I'd consider myself a hack programmer, but I would like my knowledge to be more complete than hack-level, but I don't know what I don't know so It's difficult to determine what to study given what I do know... you know?

Anyway, the way I have been typically handling this issue, is that I first start with a ThreadRegister class that is publicly static and it looks like this:

package ClassHelpers;
import javafx.beans.property.BooleanProperty;
import javafx.beans.property.SimpleBooleanProperty;
import java.util.ArrayList;
import java.util.List;
import java.util.concurrent.TimeUnit;
public class ThreadRegister {
 public static BooleanProperty keepRunning = new SimpleBooleanProperty(true);
 private static List<Long> threadIDList = new ArrayList<>();
 private static long threadID = 0;
 public static long addThread() {
 threadID++;
 threadIDList.add(threadID);
 return threadID;
 }
 public static void removeThread(Long threadID) {
 threadIDList.remove(threadID);
 }
 public static void exitApp() {
 new Thread(() -> {
 keepRunning.setValue(false);
 while (threadIDList.size() > 0) {
 try {TimeUnit.MILLISECONDS.sleep(10);} catch (InterruptedException e) {e.printStackTrace();}
 }
 System.exit(0);
 }).start();
 }
}

The publicly static BooleanProperty is then bound to the same object type with the same name in any other class that has looping threads, and such a class would look like this:

package Tests;
import ClassHelpers.ThreadRegister;
import javafx.beans.property.BooleanProperty;
import javafx.beans.property.SimpleBooleanProperty;
import java.util.concurrent.TimeUnit;
public class MyClass {
 public MyClass() {
 this.keepRunning.bind(ThreadRegister.keepRunning);
 }
 private BooleanProperty keepRunning = new SimpleBooleanProperty();
 private void myThread() {
 new Thread(()->{
 long threadID = ThreadRegister.addThread();
 while (keepRunning.getValue().equals(true)) {
 //Do necessary processing here then
 //Sleep for a short time to keep CPU consumption low
 try {TimeUnit.MILLISECONDS.sleep(50);} catch (InterruptedException e) {e.printStackTrace();}
 }
 ThreadRegister.removeThread(threadID);
 }).start();
 }
}

I always make sure that any loops within a threads' main while loop also depend on that BooleanProperty with sleep values in every loop sitting right around 50 milliseconds to make sure that when the app is closed, the threads stop quickly. And last, I set the JavaFX Stage onCloseRequest value to the ThreadRegisters exitApp() method like this:

this.stage.setOnCloseRequest(e->ThreadRegister.exitApp());

Now, no matter how many of these threads I have running, when the stage is close requested, the ThreadRegister sets the BooleanProperty to false, then it waits for all of the threads to stop running before finally exiting the app.

• SedJ601: How is this related to JavaFX?
• Michael Sims: @Sedrick It's not, but if I connected this post to JavaFX, that was my mistake.
• SedJ601: Sorry, I see where you use Properties and Stage. I guess a lot of the JavaFX code is missing. Anyway, if you are writing this in JavaFX, use Task or Service over pure Threads.

Answer 1

Hello and welcome to code review! My main question is: How often do you write programs that have several threads running throughout the application lifecycle with identical exit condition? It is possible that this might negatively affect the way you design your applications and make you use practises that aren't quite optimal? For example, instead of starting and stopping a thread when needed, do you now have threads running forever and waiting for a specific start signal?

Java already has quite powerful tools for managing concurrency, such as executors and parallel streams. I think you might benefit from studying those before rolling your own.

Code

The ThreadRegister has one glaring bug: even though it deals with threads and concurrency, it is not thread safe itself. Access to threadIDList and threadID should be synchronized.

You should follow Java naming conventions. Mainly, packages should be in lower case.

By using property-classes from the javafx package, you have created a dependency from a fairly generic looking utility to a full blown desktop application framework. This limits the usability of your utility a lot.

• Michael Sims: I was not aware that packages should be lower case, I always treated them like class names... what do yo mean by the threadIDList and threadID should be synchronized? I like the way properties from JavaFX work because you can easily communicate between class instances without having to pass the class itself onto another class ... and as far as the looping threads go, Often, they check for some event that has happened ... but sometimes they just continually "do stuff" like the thread I mentioned in my second paragraph...

Answer 2

As for a real review, see Torben's answer.

Apart from that: have a look at the method Thread.setDaemon(). Setting your worker threads to daemon would probably solve your problem without any further code. (See this question in SO https://stackoverflow.com/questions/2213340/what-is-a-daemon-thread-in-java)

If you want to perform additional parallel tasks in JavaFX specifically, have a look at Tasks and Workers. For things like transitioning a color over time, have a look at JavaFX's "animations" and "timeline animations".

• Michael Sims: I've worked with animations - specifically to make a slide-out menu drawer, but I've not seen how one can fade color using animations... but in this particular case, there is still the problem of determining when to start the fade and a thread is the only way I can think of to calculate when the number of seconds within a minute hits the 20 second mark and then again when it hits the 50 second mark as that would be when the color fade needs to start and it fades over 10 seconds each time it runs.
• Michael Sims: Concerning the setDaemon method... after reading what you posted, would it be reasonable to say that if my thread is not performing any I/O operations, then I can just set it up as Daemon and not even bother with making sure it's shut down before a System.exit() is called?
• mtj: Probably yes. But as usual, trying it out is better than discussing the theory.
• SedJ601: @MichaelSims, if you are using JavaFx and need to do any animations, you should use something from the Animation API. It will probably make what you are trying to do very easy. Also, @mtj is correct. Using Thread.setDeamon(true) should solve your problem. However, this problem should go away if you use something from the Animation API.
• Michael Sims: @SedJ601 - This is of course two years later. These days, I primarily just use simple threads or I'll utilize TimerTask with Timers which has proven to be a great way to implement threads that need to repeat on a schedule. I don't ever do animations in Java ... or anywhere for that matter. The other advantage of TimerTasks is that when you kill the app, the timer itself just stops doing what it's doing and no need to worry about threads that aren't done - when I write them, or course, so that they don't infinitely loop - which is substituted with the Timer object and is easily terminated.

Answer 3

@TorbenPutkonen's answer includes all the important facts about your code, I want just to add some minor thoughts. When you works with java threads and you have some shared variables check if there is a thread safe alternative already implemented in the java library. So instead of long and boolean use the thread safe alternatives AtomicLong and AtomicBoolean, same approach for the list that must be synchronized.

• Michael Sims: I've seen atomic variables before in example code, but I've never looked into what they are or why they exist ... I will read up on them, thank you.
• dariosicily: @MichaelSims You are welcome.