NetBeans / Java / New hint: Thread.sleep called in loop

Question

In NetBeans, there's a new hint that says: Thread.sleep called in loop.

Question 1: How/when can it be a problem to sleep in a loop?

Question 2: If it's a problem, what should I do instead?

UPDATE: Question 3: Here's some code. Tell me in this case if I should be using something else instead of Thread.Sleep in a loop. In short, this is used by a server which listens for client TCP connections. The sleep is used here in case the max number of sessions with clients is reached. In this situation, I want the application to wait until a free session becomes available.

public class SessionManager {
    private static final int DEFAULT_PORT = 7500;
    private static final int SLEEP_TIME = 200;
    private final DatabaseManager database = new DatabaseManager();
    private final ServerSocket serverSocket = new ServerSocket(DEFAULT_PORT);

public SessionManager() throws IOException, SQLException
{
}

public void listen()
{
while (true)
    if (Session.getSessionCount() < Session.getMaxSessionCount())
        try
        {
             new Thread(new Session(database, serverSocket.accept())).start();
        }
        catch (IOException ex) { ex.printStackTrace(); }
    else
        try
        {
            Thread.sleep(SLEEP_TIME);
        }
        catch (InterruptedException ex) { ex.printStackTrace(); }
}

public static void main(String[] args) throws IOException, SQLException
{
new SessionManager().listen();
}
}

Anecdotage

Answer 1

Calling sleep in a loop typically leads to poor performance. For example:

while (true) {
    if (stream.available() > 0) {
       // read input
    }
    sleep(MILLISECONDS);
}

If MILLISECONDS is too large, then this code will take a long time to realize that input is available.

If MILLISECONDS is too small, then this code will waste a lot of system resources check for input that hasn't arrived yet.

Other uses of sleep in a loop are typically questionable as well. There's usually a better way.

If it's a problem, what should I do instead?

Post the code and maybe we can give you a sensible answer.

EDIT

IMO, a better way to solve the problem is to use a ThreadPoolExecutor.

Something like this:

public void listen() {
    BlockingQueue queue = new SynchronousQueue();
    ThreadPoolExecutor executor = new ThreadPoolExecutor(
            1, Session.getMaxSessionCount(), 100, TimeUnit.SECONDS, queue);
    while (true) {
        try {
            queue.submit(new Session(database, serverSocket.accept()));
        } catch (IOException ex) { 
            ex.printStackTrace();
        }
    }
}

This configures the executor to match the way your code currently works. There are a number of other ways you could do it; see the javadoc link above.

Lambda

How about a java.util.Timer and periodic TimerTask? – Luigi

@Luigi - it depends on how you use it. If you just use it to implement the equivalent of sleep() you have the same problems as using sleep(). – Lambda

Answer 2

As others have said it depends on the usage. A legitimate use would be a program that is designed to do something every 10 seconds (but is not so critical that exact timing is needed). We have lots of these "utility apps" that import data and other such tasks every few minutes. This is an easy way to perform these tasks and we typically will set the sleep interval to be very low and use a counter so that the program stays responsive and can exit easily.

int count = 0;
while (true) {

    try {
        // Wait for 1 second.
        Thread.sleep(1000);
    }
    catch (InterruptedException ex) {}

    // Check to see if the program should exit due to other conditions.
    if (shouldExit())
        break;

    // Is 10 seconds up yet? If not, just loop back around.
    count++;
    if (count < 10) continue;

    // 10 seconds is up. Reset the counter and do something important.
    count = 0;
    this.doSomething();
}

Crave

Using a scheduled thread pool executor instead would be technically better in most cases, but that's sometimes too much when you're just programming something quick-and-dirty. – Marbles

@Marbles lets say I currently have a thread with a loop with a sleep inside. It is doing housekeeping tasks such as purging old data that isn't relevant anymore. Using a scheduled thread pool executor would add unnecessary complexity to the code, compared to just having a long lived, mostly idle thread. The context switch penalty is negligible for a long lived/idle thread that is sleeping. What does a Scheduled Thread Pool actually buy you, other than removing the "its not a good practice" warning? – Sleazy

Have you written out the code for each? I suspect that the scheduled thread pool executor version might actually be shorter, easier to read and very similar efficiency. – Marbles

Answer 3

I think I come across one completely legitimate use of sleep() method in loop.

We have one-way connection between server and client. So when client wants to achieve asynchronous communication with server, he sends message to the server and than periodically polls for some response from server. There needs to be some Timeout interval.

Response resp = null;
for (int i = 0; i < POLL_REPEAT && resp == null; i++) {
    try {
       Thread.sleep(POLL_INTERVAL);
    } catch (InterruptedException ie) {
    }
    resp = server.getResponse(workflowId);
}

POLL_REPEAT * POLL_INTERVAL ~ TIMEOUT interval

Medrano

A better solution if possible with your API would be to use a blocking IO method, that waits until the moment that new data is available. – Setscrew

Answer 4

How/when can it be a problem to sleep in a loop?
People sometimes employ it in place of proper synchronization methods (like wait/notify).

If it's a problem, what should I do instead?
Depends on what you're doing. Although it's dificult for me to imagine situation where doing this is the best approach, I guess that's possible too.

You can check Sun's concurrency tutorial on this subject.

Relly

People also sometimes use it in read loops in conjunction with testing available(), when they should just be blocking in the read instead of wasting time and space. – Canning

One usage is a directory cleaner, that deletes files older than a certain period. After the first run, we know which is the oldest file, and exactly how long until it becomes "too old", so we can sleep for that period. – Lucero

@Lucero - a scheduled thread pool executor would probably be better for that. – Lambda