"Thread already started" when resuming the activity

Question

Here is my situation: I am building a game for android and my game's activity is composed of a custom surfaceView which has a thread for the game logic and rendering. The architecture is similar to the LunarLander demo from Google's website.

When the activity starts it creates the surfaceView and calls this method:

    @Override
    public void surfaceCreated(SurfaceHolder holder)
    {   
        renderThread.start();
    }

When I press the home button to exit the game, the onPause() method is called, which calls surfaceDestroyed(). In surfaceDestroyed I stop the game Thread by calling:

    @Override
    public void surfaceDestroyed(SurfaceHolder holder)
    {
        synchronized(holder)
        {
            renderThread.stop();
        }
    }       

The app goes to background fine. Then, when I relaunch the app by pressing the icon, I get a "Thread already started" message in the log along with a "force close" popup on the screen. This message happens when the activity enters the "surfaceCreated" method when it calls start() on the render thread.

Now I've looked into it for hours and can't figure out why this is. I believe my thread is stopped when I close the app so I don't understand why it says it has already started.

Answer 1

Those methods don't do what you think they do. From the API doc:

It is never legal to start a thread more than once.

And:

public final void stop() - Deprecated. This method is inherently unsafe.

If you want to pause a thread, you have to use Object.wait() and Objecft.notifyAll() from within the thread.

Answer 2

In my opinion you should not pack your code into a subclass of Thread if you intend to be starting and stopping the Thread often (the examples do it because it makes the code shorter). Use a Runnable instead. That way you can stop and discard the old Thread when you want and create a new Thread object to execute the Runnable when you need to start it again.

private TutorialRunnable tutorialRunnable;

...

// Synchronization and error checking omitted for brevity.
public void surfaceCreated(SurfaceHolder holder) {
    thread = new Thread(tutorialRunnable);
    thread.start();
}

public void surfaceDestroyed(SurfaceHolder holder) {
    tutorialRunnable.setRunning(false);
    while (thread != null) {
        try {
            thread.join();
            thread = null;
        } catch (InterruptedException e) {
        }
    }
}

Also, relying on exceptions is bad form. That should be used as a last resort when your own code behaves unexpectedly.