Java - Thread.sleep in the main method

Question

I'm reviewing Java code that essentially is a recurring process that moves/reads/parses some files on regular basis and outputs data into the database. The repeating part is done (roughly) as follows:

public class CollectorMain {
    public static boolean signalRecieved = false;
    public static void main(String[] args) {
         Runtime.getRuntime().addShutdownHook(new Thread() {
              public void run() {  
              shutdown(); 
         }});
         while(!signalRecieved) {
              Collector.execute();
              try {
                  Thread.sleep(60 * 1000);
              } catch (InterruptedException e) {
                  break;
              }
         }
         // some shutdown logic
    }
    public static void shutdown() {
        signalReceived = true;
    }
}

public class Collector() {
    public static void execute() {
        // Move files from the queue dir to temp location
        // Read, parse files and insert records into database. 
        // Then delete the processed files
    }
}

My recommendation was to refactor code to

1. Create instance of Collector and refactor static execute() method to the instance method
2. To use Runnable or TimerTask to handle invocations

My argument was that using Thread.wait from the main method and combining it with static access is not a good way of handling repeatable process especially doing file IO. To which the author replied (quoting)

The description of Runnable says "should be implemented by any class whose instances are intended to be executed by a thread". In fact, I am intentionally avoiding threads in this program, for reasons of cost vrs.performance requirements.

Here's another quote from same discussion which will hopefully help to clarify the author's position

Technically, Java isn't executed at all, it is interpreted by the JVM which then executes machine instructions, to simulate that the Java code is executing. So it's really the JVM that is executing on a thread, or multiple threads.

But as a Java code writer, I don't care. If I don't create "threads" in Java, then It's the job of the JVM to appear as if there are no threads — even if the JVM is using threads "under the covers".

A Java Pause is not executed, it is simulated by a sequence of machine instructions that may or may not call an OS 'wait'. (It probably does, because the JVM would not want to spin, burning CPU cycles, but that's a JVM implementation choice).

So I have 2 questions:

1. Is putting Thread.wait into the main method legit, safe and advisable way of doing repeatable task in this instance? And if not, why not since there's only one (main) thread of execution?
2. What are the pitfals of using static metods in this context (if any)?

I'll be happy to provide additional info if you have any other questions.

Answer 1

You're really arguing about design decisions, not performance decisions.

Your colleague's statement about how Thread.sleep is implemented is basically incorrect as far as I can see. On a sensible JVM on a sensible operating system, Thread.sleep() is implemented using an O/S native method for parking a thread (or putting it in a "timed wait" state or whatever you want to call it on your OS). Or put another way, while a thread is sleeping, it consumes zero CPU. And in any case, TimerTask will use Thread.sleep (or similar-- I don't just recall if it uses the park() method introduced in Java 5, but it essentially makes no difference).

The JVM won't generally make secret under-the-hood decisions about threading. If you ask for another thread, you'll get one; if you don't, you won't. A few "housekeeping" threads will be created for garbage collection etc, but as far as your code is concerned, you can assume that no secret clever thread creation is going on.

So, coming back to your questions:

• sleeping in the main thread is perfectly safe per se; obviously, while the main thread is sleeping, it won't be executing anything, but if you don't need it to, then that's fine;
• whether you use Thread.sleep() directly in your code or use one of the "timer" utility methods is again a design decision based on whether you want to use a general library that removes implementational details from your code, or whether you prefer to have those details present and under your control; performance-wise, it will make little odds if you implement things correctly;
• whether you have a static method or have an instance of Collector doesn't essentially matter-- it's just a design decision that you need to make based on what seems more intuitive: do you see Collector as a "general utility" class, or as a "thing" with state that you request to perform operations?

Answer 2

I think your colleague doesn't really understand the JVM and its threading model; your code is not going to magically multi-thread unless you explicitly make it that way. Further, I think both of you are working too hard at this. Give the Quartz library a try: http://quartz-scheduler.org/documentation/quartz-2.1.x/quick-start

My reasoning is that Java Threading is difficult to get right, especially when you find yourself doing wait/notify on your own, and grappling with all of the edge cases. The quartz library has abstracted this all away and put the repetitive aspect behind the familiar CRON pattern.

I would totally avoid TimerTask as it has the nasty habit of silently failing if there are any unhandled exceptions that occur during your run.

Calling static methods or not is no big deal regardless of your solution, the key is understanding what state gets shared across threads and either eliminate the sharing OR synchronize the access to it so all Threads get a consistent view of the data. If I was in your position, I would give Quartz a shot. If you are reluctant to add yet another library, JDK 1.5 (I think) brought in the ScheduledExectutorService which I think does repetitive tasks.

No matter which way you go, don't write the scheduling/exectution framework yourself. This is a solved problem in Quartz or the ScheduledExectutorService.

Answer 3

In the code sample posted, I would use Object.wait(60 * 1000) instead of Thread.sleep(60 * 1000), then within the shutdown method, add a call to notifyAll() after setting signalReceived = true. This will require adding the necessary synchronization blocks around both the notify and the wait methods. At a minimum, this will provide the benefit of the "wait loop" being able to exit immediately, rather than waiting for the timeout to elapse first. (See JB Nizet's answer for some additional detail around this.)

From an overall perspective - I'd have the Collector implement TimerTask (sub-interface of Runnable), schedule it with a Timer, and be done with it.

Some users will advocate the use of static for performance reasons, which I would argue to be mostly historical. Keeping everything non-static will allow for future use of multiple instances within the same JVM, as well as being able to use subclasses to override methods and customize the base implementation.

Answer 4

You're confusing sleep and wait. sleep makes the current thread idle for some period of time, and then the thread restarts on its own. wait is a method of Object. It's used to put a thread in a waiting state, and it will only go out of this state if another thread wakes him up using notify or notifyAll on the same object. Using wait if there's only one thread executing will just make your program hang eternally.

Read http://docs.oracle.com/javase/tutorial/essential/concurrency/index.html