views:

305

answers:

10

Is while (true) { ... } loop in threads bad? What's the alternative?

Update; what I'm trying to to...

I have ~10,000 threads, each consuming messages from their private queues. I have one thread that's producing messages one by one and putting them in the correct consumer's queue. Each consumer thread loops indefinitely, checking for a message to appear in their queue and process it.

Inside Consumer.java:

@Override
public void run() {
    while (true) {
        Message msg = messageQueue.poll();
        if (msg != null) {
            ... // do something with the message
        }
    }
}

The Producer is putting messages inside Consumer message queues at a rapid pace (several million messages per second). Consumers should process these messages as fast as possible!

Note: the while (true) { ... } is terminated by a KILL message sent by the Producer as its last message. However, my question is about the proper way to do this message-passing...

Please see the new question, regarding this design.

A: 

Usually, you'll want to wait on a resource of some kind to do work, which hides actual threading details from you. It sounds like you wanted to implement your own spinlock.

Here's some tutorial about locking I found I google.

Stefan Kendall
+4  A: 
while (!stop_running) { ... }

...perhaps? A some sort of exit flag is often used to control thread running.

progo
If you do this, make sure stop_running uses the volatile keyword. See Java Concurrency In Practice, section 7.1.
Phil M
+4  A: 

Not inherently, no. You can always bail using break or return. Just make sure you actually do (at some point)

The problem is what happens when your thread has nothing to do? If you just loop around and around checking a condition, your thread will eat up the whole CPU doing nothing. So make sure to use wait to cause your thread to block, or sleep if you don't have anything to wait on.

Eric Petroelje
A: 

Depends on the definition of "bad". It means that the person trying to read the code has to look elsewhere for the reason that the loop is terminated. That may make it less readable.

This mentality taken to the extreme results in the COMEFROM keyword. http://en.wikipedia.org/wiki/COMEFROM

10 COMEFROM 40
20 INPUT "WHAT IS YOUR NAME? "; A$
30 PRINT "HELLO, "; A$
40 REM
Thorbjørn Ravn Andersen
+1  A: 

It's better to have the termination condition on the while (...) line, but sometimes the termination condition is something you can only test somewhere deep inside the loop. Then that's what break is for (or exceptions). In fact maybe your thread must run forever until your program terminates (with System.exit); then while (true) is definitely right.

But maybe you're asking about what should go inside the loop. You need to make sure to include some blocking operation, i.e., some function call where your thread will wait for someone else (another thread, another program, the OS) to do something. This is typically Condition.wait if you're programming with locks, or reading from a message queue, or reading from a file or network socket, or some other blocking I/O operation.

Note that sleep is generally not good enough. You can't know when other participants are going to do something, so there's no way to avoid waking up too often (thus burning up CPU time needlessly) or too seldom (thus not reacting to events in a timely way). Always design your system so that when a thread has completed its job, it notifies whoever is waiting on that job (often with Condition.signal or by joining).

Gilles
+1  A: 

Instead of looping forever and breaking or returning, you might choose to check the interrupted status.

while (!Thread.currentThread().isInterrupted()) {
    try {
        doWork();
        wait(1000);
    } catch (InterruptedException ex) {
        Thread.currentThread().interrupt();
    }
}

If your threads are tasks managed by an ExecutorService, you can have them all end gracefully simply by calling shutdownNow().

JRL
A: 

I usually go with a class attribute boolean called 'done', then the threads' run methods look like

done = false;
while( !done ) {
    // ... process stuff
}

You can then set done=true to kill the loop. This can be done from inside the loop, or you can have another method that sets it, so that other threads can pull the plug.

JustJeff
A: 

while (true) isn't bad if there is a way to exit the loop otherwise the call will run indefinitely.

For 10000 threads doing the while(true) call is bad practice...why don't you have a sleep() on the thread to allow other threads to run or an exit strategy if the thread finish running?

The Elite Gentleman
A: 

If I were do what you are talking about I would try this:

private Object lock = new Object();    

public void run(){
    while(true){
        synchronized(lock){
            Message msg = messageQueue.poll();
            if (msg != null) {
                ... // do something with the message
            }else{
                try{
                    lock.wait();
                }catch(InterruptedException e){
                    e.printStackTrace();
                    continue;
                }
            }
        }
    }
}

This allows you to make sure that you don't get any concurrent modification exepction on your messageQueue, as well as when there is no message you will not be using CPU time in the while(true) loop. Now you just need to make sure that when you do add something to your messageQueue you can call lock.notifyAll() so that the thread will know to run again.

heater
That's not so great with a shared queue.
Tom Hawtin - tackline
A: 

It looks like you are busy waiting, assuming a standard BlockingQueue. Use take instead of poll.

Other than that, for (;;) is nicer than while (true), IMO.

Tom Hawtin - tackline