Controlling thread using wait() and notify()

Question

(Problem solved, solution below)
I have 2 classes: Equip and Command. The equip is an equipment that run commands, but I need it to be able to run only 1 command at the same time. A command is a thread, that executes on the run() function, while Equip is a normal class that don't extend anything. Currently I have the following setup to run the commands:

Command class:

@Override
public void run() {
    boolean execute = equip.queueCommand(this);
    if (!execute) {
        // if this command is the only one on the queue, execute it, or wait.
        esperar();
    }
    // executes the command.....
    equip.executeNextCommand();
}


synchronized public void esperar() {
    try {
        this.wait();
    } catch (Exception ex) {
        Log.logErro(ex);
    }
}

synchronized public void continue() {
    this.notifyAll();
}

Equip class:

public boolean queueCommand(Command cmd) {
    // commandQueue is a LinkedList
    commandQueue.addLast(cmd);
    return (commandQueue.size() == 1);
}

public void executeNextCommand() {
    if (commandQueue.size() >= 1) {
        Command cmd = commandQueue.pollFirst();
        cmd.continue();
    }
}

However, this is not working. Basically, the notify() isn't waking the command thread, so it'll never execute. I searched about the wait and notify protocol, but I couldn't find anything wrong with the code. I also tried calling the wait() directly from the queueCommand() method, but then the execution of the queueCommand stopped, and it also didn't do what it was supposed to do. Is this approach correct and I'm missing something or this is completely wrong and I should implement a Monitor class to manipulate the concurrent threads?

EDIT: I solved the problem using another completely different approach, using Executors, thanks to @Gray.

Here's the final code, it might help someone someday:

Equip class:

private ExecutorCompletionService commandQueue = new ExecutorCompletionService(Executors.newFixedThreadPool(1));

public void executeCommand(Command cmd, boolean waitCompletion) {
    commandQueue.submit(cmd, null);
    if (waitCompletion) {
        try {
            commandQueue.take();
        } catch (Exception ex) {
        }
    }
}

In the Command class I just have a method to encapsulate the equip's execute method. The boolean waitCompletion is used when I need the result of the command at the same time, and instead of calling a new thread to execute it, I just execute and wait, pretending that it's executing on the same thread. This question contains a good discussion on this matter: When would you call java's thread.run() instead of thread.start()?. And yes, this is a case where it's useful to call .run() instead of .start().

Answer 1

There are a large number of race conditions that exist in your code if Command.run() is called from multiple threads. Unless this is some sort of homework question where you have to implement the code yourself, I would highly recommend using one of the Java Executors which were added in 1.6. In this case the Executors.newSingleThreadExecutor() is what you need to limit the number of running background tasks to 1. This will allow an unlimited number of tasks to be submitted to the ExecutorService, but only one of those tasks will be executing at any one time.

If you need the thread that is submitting the tasks to block when another task is already running then you would use something like the following. This sets up a pool of a maximum of 1 thread and uses a SynchronousQueue which blocks until the worker thread consumes the job:

final ExecutorService executorServer =
    new ThreadPoolExecutor(0, 1, 60L, TimeUnit.SECONDS,
         new SynchronousQueue<Runnable>());

But if that was the case then you would just call the task directly inside of a synchronized block and you wouldn't need the ExecutorService.

Lastly, for any new concurrency programmer (of any language) I would recommend that you take the time to read some documentation on the subject. Until you start recognizing the concurrent pitfalls inherent in threading even the simplest set of classes, it will be a frustrating process to get your code to work. Doug Lea's book is one of the bible's on the subject. My apologies if I have underestimated your experience in this area.