Java - Ideal use of wait and notify?

Question

This code seems to work fine so far in testing. However I am new at multithreading and want to know if this code is ideal, since I know there is a lot of "donts" regarding concurrency.
Is there a better way to make an executor for queued Runnables on a single thread? This is my first time making one so I feel inclined to believe something could be better.

public class ExplosionExecutor{
private static List<Runnable> queue= new ArrayList<Runnable>();

private static Thread thread= new Thread(new Runnable() {
    public void run() {
        while(true){
            Runnable[] queuedump;
            synchronized (queue) {
                if(queue.size()==0){
                    try {
                        queue.wait();
                    } catch (InterruptedException e){e.printStackTrace();}
                }

                queuedump= queue.toArray(new Runnable[0]);
                queue.clear();  
            }
            for(Runnable r : queuedump)
                r.run();
        }
    }

}, "Nuke Explosions");
static{
    thread.start();
}

public static void execute(Runnable command) {
    synchronized (queue) {
        queue.add(command);
        queue.notify();
    }
}

}

Answer 1

This is okay - ish.

It's best not to reinvent the wheel.

1) There are blocking queues which have methods to wait for new items and are already synchronized:

public static void main(String[] args) throws Exception {
    final BlockingQueue<Runnable> r = new LinkedBlockingQueue<>();
    final Thread t = new Thread(new Runnable() {

        @Override
        public void run() {
            while (true) {
                try {
                    r.take().run();
                } catch (InterruptedException ex) {
                    return;
                }
            }
        }
    });
    r.add(new Runnable() {

        @Override
        public void run() {
            //do stuff
        }
    });
}

2) There is the ExecutorService API which encapsulates all this behaviour:

public static void main(String[] args) throws Exception {
    final ExecutorService es = Executors.newSingleThreadExecutor();
    es.execute(new Runnable() {

        @Override
        public void run() {
            //do stuff
        }
    });
}

3) If you want to check the success of the submitted task and/or wait for a sumbitted task to finish you cannot do that using your API. With the ExecutorService you can do this very easily.

public static void main(String[] args) throws InterruptedException {
    final ExecutorService es = Executors.newSingleThreadExecutor();
    final Future<?> f = es.submit(new Runnable() {

        @Override
        public void run() {
            //do stuff
        }
    });
    try {
        //wait
        f.get();        
    } catch (ExecutionException ex) {
        //there was an exeception in the task
    }
}

A final note is that the way you have implemented your code there is no way to stop the consumer thread.

In my first example you would need to manually call t.interrupt() and because of my implementation this would case the thread to exit. In the second/third examples you would need to call ExecutorService.shutdown() to stop the consumer threads.

If you do not stop the threads then your program will not exit unless they are daemon.

Answer 2

Why are you making your own implementation? Java has a built-in ExecutorService that can run Runnables on a single thread http://docs.oracle.com/javase/7/docs/api/java/util/concurrent/Executors.html

//runs all Runnables in a single thread, one at a time
ExecutorService executor = Executors.newFixedThreadPool(1);

executor.submit(runnable);

Answer 3

Here are few improvements... Of-course if you use BlockingQueue/ExecutorService we don't need to worry about synchronization/concurrency.

1. One main issue in the code is: "r.run()" instead of new Thread(r).start().
2. Use ConcurrentLinkedQueue data structure which is Thread safe.
3. You can offer to lock/notify on any static obj/class obj, need not be on the queue, as queue is already thread safe.
4. Queue to Array conversion is not needed. iterate for queue.poll().

Also you can also use concurrent locks API (ReentrantLock or Condition classes) instead of synchronized/wait/notify.

theexamtime.com