2

I have a multithreading application as follows:

class Distrobuter:

1- assign a task to a new Thread.

if(workers.size() >= MAXTHREADSALLOWED){
  Worker worker = new Worker(task);
  Thread t = new Thread(worker);
  workers.add(t);
  t.start();
}

2- remove Threads that have finished.

Iterator<Thread> itr = workers.iterator();
while (itr.hasNext()){
    Thread t = itr.next();
    if(t.getState() == Thread.State.TERMINATED){
        itr.remove();
    }
}

class Worker:

does a certain task

================================

However, there are some Threads that get removed even when their state is not terminated.

I know threads have been finished as they don't do any activities (no print out comming from them). But for some reason it does not satisfy this condition:

(t.getState() == Thread.State.TERMINATED)

So my question is:

  1. Is it possible that once a thread reaches end of its run method, it never change state to "TERMINATED"
  2. Is there any thing I'm missing?
nafas
  • 5,283
  • 3
  • 29
  • 57
  • 1
    On a more basic level, why are you doing this by hand instead of using an executor? – chrylis -cautiouslyoptimistic- Sep 16 '15 at 10:24
  • Based on the code provided, the `Distrobuter` class doesn't seem to be doing anything to ensure/allow/wait for the threads to terminate. The loop provided just makes a pass through all of the known threads, and discards the ones that are terminated. There's nothing to indicate that it ever waits for the ones that are not. – aroth Sep 16 '15 at 10:29
  • 1
    have a look at java.util.concurrent.Executors/Executor/ExecutorService instead of coding the thread-handling manually ! What does the Code of your Workers look like ? The fact that the do not output anything does'nt mean the threads are done. They could be blocked or waiting. – André R. Sep 16 '15 at 10:32
  • @chrylis executor would have been ideal, but since working with millions, I just can't create the objects that have to wait. – nafas Sep 16 '15 at 10:33
  • 1
    @nafas you are working with millions of tasks and create a new thread for each one ? creating threads is rather expensive (http://stackoverflow.com/questions/5483047/why-is-creating-a-thread-said-to-be-expensive) ! think about using a pooled executor. – André R. Sep 16 '15 at 10:38
  • When are your running your code that does `if(t.getState() == Thread.State.TERMINATED)` ? Is it possible you're doing that just a few nano seconds before the thread actually terminates ? Is it possible the code of your worker doesn't actually finish, even though you believe they have (e.g. they block on something you havn't anticipated ?) – nos Sep 16 '15 at 10:41
  • "no print coming from them" isn't much proof that they are really finished. Use a debugger or jstack to get the stack traces for all the threads. – WillShackleford Sep 16 '15 at 10:44
  • Show more code: Those two snippets don't tell us much. What are the _names_ of the methods? (i.e., what do you expect them to do?), When are they called? What are the class/instance variables? What heppens in the `else` case in your first snippet? – Solomon Slow Sep 16 '15 at 12:24
  • Your `Distrobuter` class looks a lot like a thread pool except that it appears to create a new thread for each new task. But the whole point of thread pools is to _re-use_ threads. Creating and destroying threads is _expensive._ – Solomon Slow Sep 16 '15 at 12:27
  • @jameslarge yeah creating and destroying threads is about 100 times more expensive, but in my application its nothing really. this way I try to keep memory clean as supposedly GC will wipe off the whatever related to the destroyed thread. – nafas Sep 16 '15 at 12:29
  • @nos well what I shown above is running non-deterministically in a loop. if the condition isn't satisfied this round , it should satisfy the next round. about the inside of run, well it shouldn't get blocked. – nafas Sep 16 '15 at 12:33

0 Answers0