2 Synchronization blocks with same object

[Manimekala Velautham]

Hi,

I have one Java class and it is a singleton class. I have one thread and one servlet.

The Java class has a
1. global variable called "myList" of type ArrayList to hold Hashmaps.
2. method addListValues()- Called by the thread every 1 hr to add a HashMap to the myList.
3. method processRequest()- Called by Servlet Requests to read the value from the myList to serve the requests.

In order to protect cuncurrent access to this myList variable, I have introduced synchronized blocks in the above methods. See the code below. Will this approach solve my problem?...Please help me with your views.

    
public void addListValues(HashMap pMap) {
    synchronized(myList) {
        myList.put(pMap);
        myList.notifyAll();
    }
}
 
public HashMap processRequest(int index) {
    HashMap val = null;
    synchronized(myList) {
        while(myList.size()==0){
            try {
                myList.wait();
            }catch(InterruptedException ie){ie.printStackTrace());
        }
        val = myList.get(index);
        return val;            
    }
}

[Edward Harned]

Did you try it? Does it work?

Don't post code here you haven't run yourself.

There are much better ways of doing this in the java.util.concurrent packages. Look into blocking queues, etc.

[Ugender Rekulampally]

Manimekala,

I don't think your synchronization in methods are correct.

public void addListValues(HashMap pMap) { 
   synchronized(myList) { 
       myList.put(pMap);  
      myList.notifyAll(); 
   }
}

Here why do you have myList.notifyAll()? I am assuming you wanted this method to be accessed by one thread at time and once it is done, that is it. you don't have to call notifyAll() to tell every other thread that accessing this method is done in this scenario. jvm automatically does that.

please look into wait() and notify()/notifyAll() concepts.

And same thing with other method

public HashMap processRequest(int index) { 
   HashMap val = null;  
  synchronized(myList) {  
      while(myList.size()==0){  
          try {  
              myList.wait();    
           }
             catch(InterruptedException ie)
           {
             ie.printStackTrace());
        }        val = myList.get(index); 
       return val;   
       }

I think you should handle this logic in different way. you access this method if you have elements. so before you get here you make sure that one thread is accessing. if elements are not there, then wait() for some other thread fills in some elements in the list.

Thanks,
Ugender

[Manimekala Velautham]

Hi, Thanks for your suggestions. I understood the purpose of wait() and notify().

In this, when the the processRequest method is in progress, I dont want to add anything to to myList shared variable. The Thread which is adding a new value or changes the value in particular index in myList should wait for the processRequest method to complete.

Same way, when the thread that is executing the addValue method is in progress on the myList variable, I want the requests to wait till the thread comes out from the addValue method.

[Ugender Rekulampally]

Manimekala,
Sure there may be different ways but is your program working as you expected? we only saw just two methods and didn't see how you are handling them or calling them.

Thanks,
Ugender

[Manimekala Velautham]

Hi,

This is a servlet application. It has some HashMaps in memory, which contains key value pairs. These HashMaps are updated at every 1 hour by a thread. The servlet requests(https) will have the 'key' as query string and to fetch value from the hashmaps. The response will have the 'value'in XML format.

For the above logic, I have 3 Java Classses
1. QueryMap.java SERVLET CLASS
2. Manager.java JAVA CLASS
3. MapMonitor.java JAVA THREAD CLASS
Manager.java
-------------

    
public class Manager {
    
    static Manager manager = null;
    ArrayList dbMaps = new ArrayList();
    HashMap indexMap = new HashMap();
    String isProcessing = "None";
 
    public static Manager getInstance() {
        if(manager == null) {
            manager = new Manager();
        }
        return manager;
    }
    
    public String processRequest(String mapName, String key) {
        String val = null;
        synchronized (dbMaps) {
            if(dbMaps.size()==0 || isProcessing.equals("Thread")) {
                try {
                    dbMaps.wait();
                }catch(InterruptedException ie) {ie.printStackTrace();}
            }
            isProcessing="Request";
            if(((HashMap) dbMaps.get(Integer.parseInt(indexMap.get(pMapName).toString()))).containsKey(key)) {
                val=((HashMap) dbMaps.get(Integer.parseInt(indexMap.get(pMapName).toString()))).get(key).toString();
            }else {
                val = "NotFound";
            }
            isProcessing="None";
            dbMaps.notify();
        }
    return val;
}
 
    public void addHashMap(String mapName, HashMap map, boolean isNew) {
        synchronized(dbMaps) {
           if(isProcessing.wquals("Request")) {
               try {
                   dbMaps.wait();
               }catch(InterruptedException ie) {ie.printStackTrace();}
           }
           isProcessing="Thread";
           if (isNew) {
               dbMaps.add(map);
               indexMap.put(mapName, dbMaps.size()-1);
           } else {
               dbMaps.add(Integer.parseInt(indexMap.get(mapName).toString()), map);
           }
           isProcessing = "None";
           dbMaps.notify();
       }
}
 
}

In the above code,

the method processRequest() will be called by the servlet for each and every request to take the value from the hash map for the key requested.

the method addHashMap() will be called by the thread by every 1 hour to update or to place the new hashmaps in manager class. (maps are not going to be removed in this application)

My need is, If any request is processing, this thread has to wait for the processRequest method to complete. Same way, if the addHashMap() method is progress, the processRequest method has to wait utill it completes.

This code is working fine, but I am not able to figure out is there any problem of dead locks or other issues. Please review this code and suggest me whether my approah is correct or not.

Thanks
[/CODE]