Good practice for multi-threading

Question

I have an app where when a "game" stats, it starts a couple different threads. I start the threads like so:

Thread thread = new Thread(new Runnable()
{
    public void run()
    {
        //...
    }
});

thread.setName("killMeAtEnd");
thread.start();

Later when the game ends i have a dispose() method inside of game which sorts through all of the running Threads and ends all of the Threads that have the name "killMeAtEnd". My question is, is this good practice? My intention is to keep my app running fast and clutter free, in my experience Threads that are left "hanging" tend to slow down the phone until the app is terminated. Is there a better way to do this? Is this even worth bothering about?

EDIT:

Here is my dispose() if anyone was interested. This code is within the class Game.

public void dispose()
{
    Thread threads[] = (Thread[])Thread.getAllStackTraces().keySet().toArray();
    for(int x=0;x<threads.length;x++)
    {
        Thread thread = threads[x];
        if(thread.getName().equalsIgnoreCase(KILL))
        {
            try
            {
                thread.interrupt();
            }catch(Exception e){Log.e(Viewer.GAME,Log.getStackTraceString(e));}
            thread = null;
        }
    }
}

public static final String KILL = "endOnDispose";

Getting stacktraces is quite expensive so it would be more efficient if you would save `threads[]` while you create the threads and make it `dispose(Thread[] threads)`. Are there other threads that keep running when your game ends or is that to prevent you from killing the UI thread? — zapl, Mar 21 '12 at 01:51
When the "game" is over the Activity doesn't finish, it moves on to something else. — John, Mar 21 '12 at 01:56

score 4 · Accepted Answer · answered Mar 21 '12 at 01:46

You have the right idea, but there are some areas for improvement:

Instead of querying the system for all running threads, just add your threads to a list whenever you create them. You can then terminate all the threads you created or wait for their completion (joining).
Interrupt only interrupts a thread in a blocking state, so you need to have an additional flag that the thread checks periodically (i.e. after every "work cycle").
Catch the interrupt exception inside your thread and handle it (i.e. exit gracefully).

score 3 · Answer 2 · edited May 23 '17 at 12:23

This isn't necessarily a bad solution, but there are a few issues:

If your threads are worker threads of the sort that handle one task to completion, there is likely a better design in which the thread ends itself. In other words, there is possibly a better flow of execution, that doesn't require being killed at the end.
When you say "sort through all the running Threads...", I imagine you're looking at ALL of the running threads in the JVM? as in something along the lines of this SO question? If that's the case, why aren't you just keeping a reference to all of the threads that your game owns instead, and then specifically killing them? As opposed to just looking for "killMeAtEnd"; I can't think of how your strategy could go wrong, but it seems a bit cleaner to keep track of your threads.

It definitely IS good practice to keep threads clean. If your threads are doing something less specific-task-oriented (e.g. waiting for network io, or something), then my first suggestion is somewhat irrelevant. I would just suggest being very careful about the design of how you keep your threads clean, because errors with threads can be a big pain.

score 0 · Answer 3 · answered Mar 21 '12 at 19:34

The class ExecutorService already exists to handle this sort of problem.

Make all your threads tasks for the ExecutorService.

When the game ends, shutdown the ExecutorService using shutdownNow. This will interrupt interrupt all the threads in the ExecutorService. You can then create a new ExecutorService when you start a new game.

If the number of threads is fixed you can use Executors.newFixedThreadPool() to create the ExecutorService.

If the number of threads is variable you can use Executors.newCachedThreadPool to create the ExecutorService.

Good practice for multi-threading

3 Answers3