Where to use Thread interupt

Question

I have some old code I am working with, and I'm not too experienced with Threads (mostly work on the front end). Anyway, this Thread.sleep is causing the thread to hang and I'm unsure what to do about it. I thought about using a counter and throwing a Thread.currentThread.interupt, but unsure of where to put it or which thread it will interupt. Here is an example of the dump. As you can see the thread count is getting pretty high at 1708.

Any advice?

"Thread-1708" prio=6 tid=0x2ceec400 nid=0x2018 waiting on condition [0x36cdf000] java.lang.Thread.State: TIMED_WAITING (sleeping) at java.lang.Thread.sleep(Native Method) Locked ownable synchronizers: - None "Thread-1707" prio=6 tid=0x2d16b800 nid=0x215c waiting on condition [0x36c8f000] java.lang.Thread.State: TIMED_WAITING (sleeping) at java.lang.Thread.sleep(Native Method) Locked ownable synchronizers: - None

@Override
public void run()
{
    Connection con = null;
    int i = 0;

    while (is_running)
    {
        try
        {
            con = ConnectionManager.getConnection();

            while (!stack.isEmpty())
            {
                COUNT++;
                String line = (String) stack.pop();
                getPartMfr(line);
                try
                {
                    if (this.mfr != null && !this.mfr.equals(EMPTY_STR))
                    {
                        lookupPart(con, line);
                    }
                }
                catch (SQLException e)
                {
                    e.printStackTrace();
                }
                if (COUNT % 1000 == 0)
                {
                    Log log = LogFactory.getLog(this.getClass());
                    log.info("Processing Count: " + COUNT);
                }
            }
        }
        catch (NamingException e)
        {
            e.printStackTrace();
        }
        catch (SQLException e)
        {
            e.printStackTrace();
        }
        finally
        {
            try
            {
                ConnectionManager.close(con);
            }
            catch (SQLException e)
            {
                e.printStackTrace();
            }
        }


        try {
            Thread.sleep(80);

        } catch (InterruptedException e) {
            // TODO Auto-generated catch block
            e.printStackTrace();
        }

    }
    this.finished = true;

}

Here is where it calls the run method, as you can see it does set it to false, but I guess it is missing threads?

    HarrisWorker w[] = new HarrisWorker[WORKER_POOL_SIZE];
    try
    {
        for (int i = 0; i < w.length; i++)
        {
            w[i] = new HarrisWorker(pw);
            w[i].start();
        }
        pw.println(headers());
        File inputDir = new File(HARRIS_BASE);
        String files[] = inputDir.list();
        for (String file : files)
        {

            try
            {
                File f = new File(HARRIS_BASE + File.separator + file);
                if (f.isDirectory())
                    continue;
                final String workFile = workDir + File.separator + file;
                f.renameTo(new File(workFile));

                FileReader fr = new FileReader(workFile);
                BufferedReader br = new BufferedReader(fr);
                String line = br.readLine();

                boolean firstLine = true;

                while (line != null)
                {
                    if (firstLine)
                    {
                        firstLine = false;
                        line = br.readLine();
                        continue;
                    }
                    if (line.startsWith(","))
                    {
                        line = br.readLine();
                        continue;
                    }
                    //          if(line.indexOf("103327-1") == -1)
                    //          {
                    //              line = br.readLine();
                    //              continue;
                    //          }
                    HarrisWorker.stack.push(line);
                    line = br.readLine();
                }
                br.close();
                fr.close();
                for (int i = 0; i < w.length; i++)
                {
                    w[i].is_running = false;

                    while (!w[i].finished)
                    {

                        Thread.sleep(80);   
                    }

                }
                move2Processed(file, workFile);
                long etime = System.currentTimeMillis();
                System.out.println("UNIQUE PARTS TOTAL FOUND: " + HarrisWorker.getFoundCount() + " of " + HarrisWorker.getUniqueCount() + ", "
                        + (HarrisWorker.getFoundCount() / HarrisWorker.getUniqueCount()));
                System.out.println("Time: " + (etime - time));


            }
            catch (Exception e)
            {
                e.printStackTrace();
                File f = new File(workDir + File.separator + file);
                if (f.exists())
                {
                    f.renameTo(new File(HARRIS_BASE + File.separator + ERROR + File.separator + file));
                }

            }
        }
    }

Why do you want to interrupt it? You are seeing it in sleep because that is where it should spend most of it's time - asleep. The name "Thread-1708" does not mean there are 1708 threads running. — OldCurmudgeon
Well, after about a week there gets to be so many threads that it bogs down our server and we have to dump and restart. Our server admin just said there are thousands of threads sleeping, I just wanted a way to close them out after a certain amount of time or something. If it would help I could send you the full thread dump, I've tried reading up on threading but it's still a little confusing to me. — Jackson Bray
In that case just set the run instance variable to false and the threads will close and terminate. Your difficulty is probably finding the right place in code to do that - start with where these threads are created and why. Better still - consider using a thread pool. — OldCurmudgeon
I updated my code to include the one that creates the worker threads. — Jackson Bray
Make sure is_running is marked volatile. Use Thread.join() instead of your strange loop looking for finished. Log every creation of a thread and every completion - your problem should become clearer. Consider using a Thread Pool. — OldCurmudgeon

Ordous Ordous · Accepted Answer · 2015-02-26T15:51:16

As a direct answer to the question in your title - nowhere. There is nowhere in this code that needs a Thread.interrupt().

The fact that the thread name is Thread-1708 does not necessarily mean there are 1708 threads. One can choose arbitrary names for threads. I usually include the name of the executor or service in the thread name. Maybe 1600 are now long stopped and there are only around a hundred alive. Maybe this particular class starts naming at 1700 to distinguish from other uses.

1708 threads may not be a problem. If you have a multi-threaded server that is serving 2000 connections in parallel, then it certainly expectable that there are 2000 threads doing that, along with a bunch of other threads.

You have to understand why the sleep is there and what purpose it serves. It's not there to just hog memory for nothing.

Translating the code to "plaintext" (btw it can be greatly simplified by using try-with-resources to acquire and close the connection):

Acquire a connection
Use the connection to send (I guess) whatever is in the stack
When failed or finished - wait 80ms (THIS is your sleep)
If run flag is still set - repeat from step 1
Finish the thread.

Now reading through this, it's obvious that it's not the sleep that's the problem. It's that the run flag is not set to false. And your thread just continues looping, even if it can't get the connection at all - it will simply spend most of its time waiting for the retry. In fact - even if you completely strip the sleep out (instead of interrupting it mid-way), all you will achieve is that the Threads will start using up more resources. Given that you have both a logger and you print to stdout via printStackTrace, I would say that you have 2 problems:

Something is spawning threads and not stopping them afterwards (not setting their run flag to false when done)
You are likely getting exceptions when getting the Connection, but you never see them in the log.

It might be that the Thread is supposed to set it's own run flag (say when the stack is drained), but you would have to decide that yourself - that depends on a lot of specifics.

Where to use Thread interupt

2 Answers