0

I'm having this odd behaviour. My program hangs eventhough I'm setting a time to force terminate it. could anyone point out what this strange behaviour could be?

here is my code where I start the threads

protected void pendingTaskStarter() throws Exception { ExecutorService service = Executors.newFixedThreadPool(maxThreadNum); ArrayList<Future<Runnable>> futures = new ArrayList<Future<Runnable>>(); System.out.println("max thread num: " + maxThreadNum); for(int i=0;i<maxThreadNum;i++){ Thread t = new PendingTaskConsumer(); Future<?> f=service.submit(t); futures.add((Future<Runnable>) f); } for (Future<?> future:futures) { future.get(5l,TimeUnit.MINUTES); // maximum thread's life is 5min (5l <-- 5 in long ) } service.shutdownNow(); }

I am 100% sure that my program hangs somewhere in PendingTaskConsumer class based on the outputs within PendingTaskConsumer class.

Anyways codes in PendingTaskConsumer should be irrelevant as thread is supposedly forced to be terminated. My question is in what scenraios the following line does not do as its expected.

future.get(5l,TimeUnit.MINUTES);

The program is running on Linux(Ubuntu) and using openjdk-7-jdk backage ( version 1.7)

Improve this question
9
  • How long is the program expected to run for? How long have you waited for it to complete? Commented Mar 13, 2015 at 13:33
  • I have an API on top of this layer, where if this program doesn't terminate within 20min, then it reboots the machine that this program is running Commented Mar 13, 2015 at 13:34
  • (1) Why is PendingTaskConsumer derived from Thread? It is not a thread, but a task that is executed by ANOTHER thread maintained by the ExecutorService. (2) is PendingTaskConsumer() a method call? If no, it is probalby missing the new keyword. Commented Mar 13, 2015 at 20:12
  • @isnot2bad PendingTaskConsumer is class, in the question I missed putting keyword "new" before, Commented Mar 16, 2015 at 9:19
  • @nafas OK. But still you should not derive from Thread here! PendingConsumer is just a Runnable or Callable, not a thread by its own! (The threads are managed by the executor internally!) Commented Mar 16, 2015 at 9:34

2 Answers 2

Reset to default
1

As @GPI already stated in his/her answer, cancellation won't work properly, if your tasks do not repond to thread interruption via Thread.interrupt(). See Oracle: The Java™ Tutorials - Concurrency - Interrupts or Java Executors: how can I stop submitted tasks?.

But even if your tasks do not immediately stop if the thread is interrupted (=the interruption flag is set), you can timeout your method. Just don't listen to task completion by invoking every single get() method of your Futures! That's not necessary as you are not really interested in their result. Just shutdown your ExecutorService and wait for its termination.

protected void pendingTaskStarter() throws InterruptedException { ExecutorService service = Executors.newFixedThreadPool(maxThreadNum); for (int i = 0; i < maxThreadNum; i++) { service.submit(new PendingTaskConsumer()); } // Shutdown service. // This will continue to process already submitted tasks. service.shutdown(); try { // wait at least 5 minutes for the tasks to complete if (!service.awaitTermination(5, TimeUnit.MINUTES)) { // still not done yet -> force shutdown // this will interrupt currently running tasks. service.shutdownNow(); } } catch (InterruptedException ex) { // someone interrupted this thread. Shutdown tasks immediately service.shutdownNow(); // propagate exception throw ex; } }

So what happens here?

  • First, all tasks are submitted.
  • Then shutdown() is called on the ExecutorService. Note that the service will continue to execute already submitted tasks, but it will not accept new tasks.
  • Now, we wait for the service to terminate by calling awaitTermination with a timeout of 5 minutes. If this method returns true, which means, that the service has terminated within time, everything is fine and we're done. But if it returns false after 5 minutes, it means that the service is still running because there are some tasks that need more time to complete.
  • In that case, we shutdown the service immediately by calling shutdownNow(). Note that this method will not block! It just attempts to cancel all running tasks, usually by interrupting the threads that are currently executing them. This is exactly why your tasks should response to thread interruption properly! If they don't do this, interrupting the thread will have no effect at all!
  • No matter if shutdownNow was successful - we are done and do not wait any longer. So there might still be some tasks that are running because they don't react upon thread interruption (usually because a task is either not implemented properly or it is uninterruptibly on purpose). But we simply don't care.
  • Note the exception handler: When another thread interrupts the thread that is calling our method while it is waiting in method awaitTermination, an InterruptedException is thrown. We handle it by shutting down the service and re-throwing the exception! That is a good thing and makes our method a long-running-operation that is cancellable by itself!
Improve this answer
Sign up to request clarification or add additional context in comments.

5 Comments

it's a very clear explanation. I like the part that you mention there is no point of keeping track of the future. I'll try it and let you know
well I'll be damn surprised , it did the trick. I believe throw ex; did the trick. even though the threads are not getting terminated. I get out of that method. then once everything else carried out all I needed to do was to add System.exit(0); to force exit the program.
@nafas As I mentioned, the method will terminate after at least 5 minutes no matter if the tasks are running or not. In your case it seems they are still running indeed - that's why the system will not halt without System.exit(0)! You should really check your task implementation and ensure, thread interruption is handled properly!
throw ex is not the important part here as it will only be called if another thread interrupts the current thread. In your case, awaitTermination just returns false and the method exists after calling shutdownNow which seems to have no effect because your tasks ignore the cancellation!
I know System.exit(0) is a very bad practice. all I wanted to do was to get out of that part of the program(which thanks to your solution I do). once I'm out of that bit, I can talk to mother server. this is an extremely heavy computation which requires ~500 servers. so previously the efficiency was at 60% now the efficiency is at 95%
1

Well... proper exception handling seems to be missing.

/** * Waits if necessary for at most the given time for the computation * to complete, and then retrieves its result, if available. * * @param timeout the maximum time to wait * @param unit the time unit of the timeout argument * @return the computed result * @throws CancellationException if the computation was cancelled * @throws ExecutionException if the computation threw an * exception * @throws InterruptedException if the current thread was interrupted * while waiting * @throws **TimeoutException if the wait timed out** */ V get(long timeout, TimeUnit unit) throws InterruptedException, ExecutionException, TimeoutException;

If your call to future.get(5l,TimeUnit.MINUTES); expires, then an Exception is thrown and your service is never shut down.

So (one can assume) its internal threads are still running, at least the one that is executing your long running task). Seeing those are non-daemons thread (you'd have to custom the ThreadFactory of your executor for them to be), those pending threads prevent the JVM from shutting down.

If you wish to terminate the tasks forcibly, you can :

  1. design your runnable task to respond to Thread.isInterrupted(), and exit when it sees that become true. (The practical effect of shutdownNow is to set the interrupted flag of your executor's thread )
  2. If you can't (because your are blocked in some library's method that does not listen to interruption), then having a dameon thread seems like the only option.

But anyway, I'd put the executor's shutdown either earlier, or in a finally clause.

Improve this answer

1 Comment

very interesting mate, I never considered this. But I'll try it and let you know.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.