using join() in run() method

[_neo_]

Hi folk.
Can I use join() method in implementation of run(), like this:

public class MyThread extends Thread {
  @Override
  public void run()
  {
     int timeout = 30000;
     // here is some heavy work
     join(timeout);
  }
}

Is above thread terminated after timeout milliseconds if task takes longer time?

I'm planning to use this thread with thread pool and want to terminate it if task takes longer time than timeout.

ExecutorService executor = Executors.newFixedThreadPool(10);
MyThread mythr = new MyThread();
executor.execute(mythr);

Please, give me some suggestions...

[Majestics]

Seems like u want to abort a thread cleanly if it fails to complete in a desired time. I think "interrupt" is one of the possible way's, u need a exception handler in which u can print a message or empty as u desire....

[JamesCherrill]

The join statement won't be executed until "some heavy work" is finished anyway, so the timeout won't be useful. I think you need to start a second thread (or just a Timer) that waits for timeout milliseconds, and use that to terminate your work thread if necessary.

[JamesCherrill]

Also - you can't just stop a thread that's busy doing stuff. Here's a really good article on how to interrupt an running thread:
http://www.techrepublic.com/article/interrupting-java-threads/5144546

[DavidKroukamp]

The best i can suggest is using a shared variable, thats either true if the thread must continue its work or false if not?

and then let your thread continuelly check that value and when it changes then it must quit

i.e

while(runthread) {
//do work
//more work
//done work
runthread=false;
}
//clean up here.

now how to end this from outside the thread? just alter the variable in your program you may use the isAlive() method to see if the thread is still busy and change it.

[_neo_]

thank you all guys.
I've accounted your suggestions and written test code. I put it here, maybe someone find it useful

This is main point:

package test;

public class Test {

	public static void main(String[] args) {
		try
		{
			TaskManager mngr = new TaskManager();
			for (int i = 1; i <= 7; i++) {
				Task tsk = new Task(10000);
				mngr.startTask(tsk);
			}
			
			while(true) {
				Thread.yield();
				mngr.checkDone();
				if (mngr.executor.isShutdown()) break;
			}
		} catch (Exception error) {
			error.printStackTrace();
		}
	}
}

These are implementation of classes

package test;

import java.util.*;
import java.util.concurrent.*;

public class TaskManager {
	private static final int NTHREADS = 5; // max active threads count
	public ExecutorService executor;
	private List<FTask> taskFutures;
	
	public TaskManager()
	{
		executor = Executors.newFixedThreadPool(NTHREADS);
		taskFutures = new ArrayList<FTask>();
	}
	
	public void startTask(Task task)
	{
		try
		{
			Callable<Task> t = task;
			Future<Task> fut = executor.submit(t);
			taskFutures.add(new FTask(fut, task.timeout));
			System.out.println("ADDED");
		} catch (Exception error)
		{
			error.printStackTrace();
		}
	}
	
	public synchronized void checkDone() {
		System.out.println("count="+taskFutures.size());
		try
		{
			if (taskFutures.isEmpty()) {
				executor.shutdown();
			}
			
			FTask t = null;
			for (Iterator<FTask> it = taskFutures.iterator(); it.hasNext(); )
			{
				try
				{
					t = it.next();
					Task task = t.future.get(5, TimeUnit.MILLISECONDS);
					// TODO: save task result on DB
					System.out.println("Task=" + task);
					it.remove();
				} catch (TimeoutException toe)
				{
					if (t.isTimedOut())
					{
						if (t.cancel()) {
							it.remove();
						}
					}
				}
			}
		} catch (Exception error)
		{
			error.printStackTrace();
		}
	}
}

package test;

import java.util.*;
import java.util.concurrent.*;

public class Task implements Callable<Task> {

	public int timeout;
	public int taskId;
	public int result;
	
	public Task(int timeout)
	{
		this.timeout = timeout;
	}
	
	public Task call()
	{
			this.taskId = (new Random()).nextInt(100);
			this.result = (new Random()).nextInt(100);
			
			int slp = (new Random()).nextInt(60000);
			System.out.printf("Sleeping for %d ms.\n", slp);
			System.out.flush();
			try {
				Thread.sleep(slp);
			} catch (InterruptedException e) {
				Thread.currentThread().interrupt();
				System.out.println("----- Stopping...");
			}
			return this;
	}
	
	@Override
	public String toString()
	{
		String x = String.format("taskId=%d, result=%d", taskId, result);
		return x;
	}
}

package test;

import java.util.*;
import java.util.concurrent.*;

public class FTask {
	
	public Future<Task> future;
	public Calendar startTime;
	public int timeout;
	
	public FTask(Future<Task> future, int timeout)
	{
		this.future = future;
		this.startTime = Calendar.getInstance();
		this.timeout = timeout;
	}
	
	/**
	 * Determines timeout is occurred or no.
	 * @return true, if timed out.
	 * */
	public boolean isTimedOut()
	{
		long diff = Calendar.getInstance().getTimeInMillis()-startTime.getTimeInMillis();
		boolean b = diff>timeout;
		//System.out.printf("Difference: %d Condition: %b\n", diff, b);
		return b;
//		return (Calendar.getInstance().getTimeInMillis()-startTime.getTimeInMillis() > timeout);
	}
	
	/**
	 * Try to cancel task.
	 * @return true, if cancellation success.
	 * */
	public boolean cancel()
	{
		try
		{
			return this.future.cancel(true);
		} catch (Exception error)
		{
			error.printStackTrace();
			return false;
		}
	}
}

[~s.o.s~]

I don't think this solves anything if your purpose here is to "cancel" the underlying computation. Create a task which continuously (while true) prints "Hello world" to the console (STDOUT) every second and set the timeout for that task to be 5 seconds. You'll notice that even after the task has been cancelled, you still get "Hello world" printed to the console. Why? Because, cancel() on Future *attempts* to cancel the underlying computation. Sure, you have expressed your desire that you don't need the future, but this doesn't mean that the "underlying" processing has stopped.

To summarise, unless your "task" is thread interrupt aware, calling cancel(true) has no effect on the computation. If you are iterating over a *large* number of element, with some processing done for each element, you are in luck since you have time window for polling the interrupt status of the thread. If you are invoking a blocking call like doing traditional I/O, you are out of luck and the only way would be to forcefully close the I/O stream/handle.

[_neo_]

~s.o.s~ you are right, thanks you for description. I replace my code as you said and even I cancel all threads, text prints to the console. Can you give some example code how to achieve correct solution?

[_neo_]

My call() method now is as below:
If I put sleep() I can interrupt it, but without it can't. ~s.o.s~ please, give me some correct example.

public Task call()
	{
			try {
				while (true) {
					Thread.sleep(1); // without sleep thread continues its work
					System.out.println("Hello world");
				}
			} catch (InterruptedException e) { // catch interrupting (cancelling) the task
				Thread.currentThread().interrupt();
				System.out.println("----- Stopping...");
			} finally {
				return this;
			}
	}

[~s.o.s~]

From your first post, drop the extends Thread. If you want to submit a task to the pool, just make your task implement Runnable or Callable instead of extending Thread.

Also, a clarification - you are not cancelling "threads" but cancelling the "future". This is a big difference because if you use a fixed thread pool, a single thread will be used to execute multiple tasks and in turn return multiple futures.

Now, the simplest solution to actually stop a computation is to "listen" to interrupts.

public Task call() {
  while (true) {
    if (!Thread.currentThread().isInterrupted()) {
      // do something
      System.out.println("Hello world");
    }
  }
  return this;
}

The reason it works if you put sleep() is because, sleep is a blocking call which throws an interrupted exception when the sleeping thread is interrupted from outside. Though sleep works, it's not a good solution because it most probably toggles the thread state as opposed to just checking the interrupt flag.

[JamesCherrill]

~s.o.s~ is there any particular reason for not using

while (!Thread.currentThread().isInterrupted()) {
  // do something

[~s.o.s~]

Ah, no particular reason. I was assuming the actual code has a real iteration over some items (as opposed to while true) in which case it's better to put the interrupt check in a separate IF statement inside the outermost loop.

[JamesCherrill]

OK, thanks.

[_neo_]

Thank you very much, ~s.o.s~, JamesCherrill.
I've modified my code.