All Threads in Blocked State ( Program Hangs )

[Akash Ashok]

Hi All,

I have a basic data structure FileQueue which contains a linked list and a hash map. I am inserting data into to it as a single thread .. but multiple threads access popFile and processed methods which are synchronized. I have an issue with this .. and at sometime all threads go into blocked state and the program hangs . I am not able to figure out why could some1 help me out.

import java.util.Collections;
import java.util.Iterator;
import java.util.LinkedList;
import java.util.HashMap;
import java.util.Vector;
import java.util.Map.Entry;


public class FileQueue {
        private LinkedList<Integer> nextFile;
        private HashMap<Integer,LinkedList<String>> terminalQueue;
        int fileCount;

        FileQueue(){
                fileCount = 0;
                nextFile = new LinkedList<Integer>();
                terminalQueue = new HashMap<Integer,LinkedList<String>>();
        }

        FileQueue(Vector<String> files){
                this();
                Collections.sort(files);
                Iterator<String> it = files.iterator();
                while(it.hasNext())
                        this.pushFile(it.next());
        }

        public boolean hasFiles(){
                return (fileCount>0)?true:false;
        }

               
        public void pushFile(String filePath){
                int lastSlashIndex = filePath.lastIndexOf("/");
                String fileName = filePath.substring(lastSlashIndex+1);
                int terminalId = Integer.parseInt(fileName.substring(19,fileName.lastIndexOf("_")).trim());
                LinkedList file = terminalQueue.get(terminalId);
                if(file==null) {
                        synchronized(this){
                                nextFile.addLast(terminalId);
                                LinkedList<String> newList = new LinkedList<String>();
                                newList.addLast(filePath);
                                terminalQueue.put(terminalId,newList );
                                fileCount++;
                        }
                }
                else {
                        synchronized(this){
                                terminalQueue.get(terminalId).addLast(filePath);
                                fileCount++;
                        }
                }
        }

        public synchronized void processed(int terminalId)throws InterruptedException{
                if(terminalQueue.containsKey(terminalId))
                        nextFile.addLast(terminalId);
                notify();

        }


          public synchronized String popFile() throws InterruptedException{
                        if(nextFile.size()==0) wait();
                        int terminalId = nextFile.poll().intValue();
                        LinkedList<String> queueFileNames = terminalQueue.get(terminalId);
                        fileCount--;
                        String retValue = queueFileNames.poll();
                        if(queueFileNames.size()==0)
                                terminalQueue.remove(terminalId);
                        return retValue;

        }

In the main program the code is being called this way

public void run(){
     while(fileQueue.hasFiles()){
                  String retname = fileQueue.popFile();
                  // Process file with name stored in retname 
    }
}

[Stephan van Hulst]

Hello Akash, welcome to CodeRanch.

It looks like all the threads are waiting at popFile(). Can you guarantee that there are other threads calling processed() to wake them up? If not, you have guaranteed deadlock.

Also, you are calling notify(). Don't call notify(). Use notifyAll() instead.

[edit]

You may also want to use a while statement to check whether wait() should be called. Never call wait() from an if statement, it's not reliable.

[Chris Hurst]

- Never call wait outside a loop, a good bug finder program will flag that one ..

wait description

- Reads of fileCount should be synchronized or you may not see the current value (ever) i.e. synchronize hasFiles ()

- pushFile calls get on terminalQueue with no memory barrier, HashMap isn't thread safe, reads AND writes should be from sync blocks if you want to use multiple threads !


Can I suggest you simply the whole thing down to synchronizing all the methods maybe (I'd avoid small sync blocks of code as invariably the author gets it wrong, yes I know performance but thats only useful if your code is correct), could you not just synchronize pushFile if you get dead lock you can get a stack trace from the OS that will list all the threads and why they're blocked <ctrl><break> Windows, post that and it should be a lot more straight forward to see why your deadlocked.

[Akash Ashok]

Hi,
Thanks a lot !!!
I put the wait() inside while loop and notifiedAll().
The program pretty much started workin fine except for a deadlock on a DB connection which is unrelated to this. Thanks a lot of the help