Java: Is `while (true) { ... }` loop in a thread bad? What's the alternative?

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

Update; what I'm t开发者_JAVA技巧rying 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.

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().

while (!stop_running) { ... }

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

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.

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

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.

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).

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?

First up, the straight answer to this problem by Dough Lea:

It is almost never a good idea to use bare spins waiting for values of variables. Use Thread.onSpinWait, Thread.yield, and/or blocking synchronization to better cope with the fact that "eventually" can be a long time, especially when there are more threads than cores on a system.

http://gee.cs.oswego.edu/dl/html/j9mm.html

Thead.onSpinWait was introduced with Java 9. It could look like this.

while (true) {
    while (messageQueue.peek() == null) {
       Thread.onSpinWait();
    }
    // do something with the message
}

By invoking this method within each iteration of a spin-wait loop construct, the calling thread indicates to the runtime that it is busy-waiting. The runtime may take action to improve the performance of invoking spin-wait loop constructions.

https://docs.oracle.com/javase/9/docs/api/java/lang/Thread.html#onSpinWait--

Although all the above answers are correct, I want to suggest this one as I came across this situation myself: You can use a flag say:

isRunning=true;
while(isRunning){
   //do Something
}

Later, make sure that isRunning is set to false after you are done reading from the buffer or data file.

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.

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.

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.