does anyone see any issues with this thread pattern?

Question

Here is a simple thread pattern that I use when writing a class that needs just one thread, and needs to a specific task.

The usual requirements for such a class are that it should be startable, stopable and restartable. Does anyone see any issues with this pattern that I use?

public class MyThread implements Runnable {
    private boolean _exit = false;
    private Thread _thread = null;

    public void start () {
        _exit = false;

        if (_thread == null) {
            _thread = new Thread(this, "MyThread");
            _thread.start();
        }
    }

    public void run () {
        while (!_exit) {
            //do something
        }
    }

    public void stop () {
        _exit = true;

        if (_thread != null) {
            _thread.interrupt();
            _thread = null;
        }
    }
}

I am looking for comments around if I am missing something, or if there is a better way to write this.

Answer 1

I would advise against use of the Thread class directly. The Executors framework available since Java 5 simplifies a lot of the issues involved in threading. The idea is that your class would perform the task required, and all of the threading functionality is managed by an Executor, saving you the effort of dealing with the complexity of threading.

A good intro can on the Java Executors framework can be found here.

Answer 2

Well, the class itself is not thread safe. That's not necessarily a problem as long as that's documented and observed in code. If it's not you could lose references to Thread objects that will run in parallel, if two consumers get inside the start() method at the same time.

Flags used as semaphores should also of course be made volatile.

The API of the class is a little bit strange. You implement Runnable, saying to other classes "use my run method to invoke me" but then mimic the start method of a full Thread object. You may want to hide the run method inside an inner class. Otherwise it's somewhat confusing how one is intended to use the object.

And as always, any pattern that involves the words new Thread() rather than using a pool is somewhat suspect in the abstract. Would need to know about what you're actually doing with it to really comment intelligently on that though.