Multithreading correctly done? I rarely write multithreaded code, and am on shaky ground when doing so.  I make mistakes.  I would like a sanity check of something I am going to introduce in one of my apps.

There will be exactly one running instance of the Thread class below in my application.  (It will be stored in a servlet context attribute.)  "Clients" will call one of three methods on the instance, getRunList() to get data from it, signalFreshRunListCreation() to tell the thread to generate new data, and stopThread() to signal the thread to exit.  On its own, the thread creates new data every 30 seconds.

Have I made any errors?  


public class ThreadRunListDefault extends Thread {

    // This set to true signals that the thread should exit.
    private boolean _boStop = false;
    // The thread sets this to true just before it sleeps.
    private boolean _boIsSleeping = false;
    // An external client sets this to true to signal that a new run list should  be created.
    private boolean _boFreshRunListRequested = false;
    // This is a mutex to lock the run list while a fresh one is being created.
    private Object _lockRunList = new Object();
    // This is the actual data that will be returned.
    private List&lt;BeanRunOutput> _runList;
    // This is the time the thread will sleep between refreshing the data on its own
    private long _iSleepMilliseconds = 30000;

    @Override
    public void run() {

        // Loop as long as no client has indicated the thread should stop. 
        while (!_boStop) {
            // Create the data.
            this.createFreshRunList();

            // Only sleep if a refresh of the data wasn't requested while we were 
            // creating a fresh run list.
            if (!_boFreshRunListRequested) {
                try {
                    // We set the sleeping flag to true so that signalFreshRunListCreation() will 
                    // know whether or not to call interrupt() on the thread and wake it up.
                    _boIsSleeping = true;
                    Thread.sleep(_iSleepMilliseconds);
                } catch (InterruptedException e) { }
                _boIsSleeping = false;
            } 
        }
    }

    /*
     * This is the method that will be called by run() to create new data in the _runList member variable.
     */
    private void createFreshRunList() {
        synchronized(this._lockRunList) {
            // Here we put the code that creates the run list.  
            // Typically it will take 5 to 10 seconds.
            _runList = new ArrayList&lt;BeanRunOutput>();
        }
    }

    /*
     * This is the method a client will call to get the data in _runList.
     */
    public List&lt;BeanRunOutput> getRunList() {
        synchronized(this._lockRunList) {
            return _runList;
        }
    }

    /*
     * This is the method a client will call to signal the thread that a new run list should be created.
     * If run() is currently gathering data (_boIsSleeping is false), a flag will be set to tell run() 
     * to run again immediately after it is finished, without sleeping first.  If the thread is currently
     * sleeping, it will be interrupted. 
     */
    public void signalFreshRunListCreation() {
        if (_boIsSleeping) {
            // Thread is sleeping so interrupt it.
            this.interrupt();
        } else {
            // The thread is running so signal it should just run again when 
            // complete (in other words not sleep).
            _boFreshRunListRequested = true;
        }
    }

    /*
     * This method allows a client to signal the thread to exit.
    */
    public void stopThread() {
        _boStop = true;
    }
}
The  _boStop and the _boIsSleeping fields are accessed by multiple threads, so you should synchronize them. You could use volatile fields or AtomicBooleans too.

Make sure that you do read operations in synchronized blocks too, not just writes:


  [...] synchronization has no effect unless both read and write operations are synchronized.


From Effective Java, 2nd Edition, Item 66: Synchronize access to shared mutable data.
A possible bug: _boFreshRunListRequested is never set to false.
Using _ prefixes (and sometimes this.) to access fields it's not really necessary and it's just noise. Modern IDEs use highlighting to separate local variables from instance variables.
You could eliminate some comments, for example:

// Loop as long as no client has indicated the thread should stop.
while (!boStop) {


This could be

while (!threadStop) {


Another example:

   // Create the data.
   this.createFreshRunList();


Here you could rename the method to createData or createFreshData.

A good reading in this topic is Clean Code by Robert C. Martin and Code Complete, 2nd Edition by Steve McConnell.
It mainly depends on your requirements, but I'd consider renaming the class to DataRefresherImpl and implementing two interfaces: DataRefresherService and  DataRefresher. The first would be used only in the servlet/container initializer. The second is for the clients to get their data and to request new data creation. It would prevent clients to call the stopThread method accidentally.
OldCurmudgeon has already mentioned the getRunList method is dangerous. You could use Collections.unmodifiableList here. Furthermore, make sure that the BeanRunOutput objects are also immutable or handle the modifications. See also: Effective Java, 2nd Edition, Item 39: Make defensive copies when needed


Finally, here is an refactored version with the queue which was suggested by OldCurmudgeon.

public interface DataRefresherService {
    void shutdown();
}




public interface DataRefresher {
    List&lt;BeanRunOutput&gt; getRunList();
    void signalFreshRunListCreation();
}




public class DataRefresherImpl implements Runnable, 
        DataRefresherService, DataRefresher {

    private final AtomicBoolean threadStop = new AtomicBoolean(false);

    private final ReentrantReadWriteLock listLock = new ReentrantReadWriteLock();

    private static final long REFRESH_INTERVAL_IN_MILLIS = 3000;

    private final ArrayBlockingQueue&lt;String&gt; refreshQueue = 
        new ArrayBlockingQueue&lt;String&gt;(1);

    private List&lt;BeanRunOutput&gt; runList;

    public DataRefresherImpl() {
    }

    @Override
    public void run() {
        while (!threadStop.get()) {
            createFreshRunList();
            sleepAndWakeOnRequest();
        }
    }

    private void createFreshRunList() {
        listLock.writeLock().lock();
        try {
            runList = new ArrayList&lt;BeanRunOutput&gt;();
        } finally {
            listLock.writeLock().unlock();
        }
    }

    private void sleepAndWakeOnRequest() {
        try {
            refreshQueue.poll(REFRESH_INTERVAL_IN_MILLIS, TimeUnit.MILLISECONDS);
        } catch (InterruptedException ignored) {
        }
    }

    @Override
    public List&lt;BeanRunOutput&gt; getRunList() {
        listLock.readLock().lock();
        try {
            return runList;
        } finally {
            listLock.readLock().unlock();
        }
    }

    @Override
    public void signalFreshRunListCreation() {
        refreshQueue.offer("refresh");
    }

    @Override
    public void shutdown() {
        threadStop.set(true);
        refreshQueue.offer("shutdown");
    }
}


(I've not tested this too much.)