aspose file tools*
The moose likes Java in General and the fly likes Collections.synchronizedMap Help Big Moose Saloon
  Search | Java FAQ | Recent Topics | Flagged Topics | Hot Topics | Zero Replies
Register / Login


Win a copy of Java 8 in Action this week in the Java 8 forum!
JavaRanch » Java Forums » Java » Java in General
Bookmark "Collections.synchronizedMap Help" Watch "Collections.synchronizedMap Help" New topic
Author

Collections.synchronizedMap Help

Michael Melusky
Greenhorn

Joined: May 11, 2009
Posts: 5
Hi, we're using this construct to store data as cache within our application. We're initializing the object as follows:



We have this method then that retrieves data from the cache:



This private function simply adds an object to the ArrayList (I removed the business logic.)



In regards to my question, is there any way that the getValues() function can return faulty data? We're receiving inconsistent results, and I'm concerned as to whether I'm not synchronizing the map enough. We're working in 1.4. Thank you for any assistance.

Paul Clapham
Bartender

Joined: Oct 14, 2005
Posts: 18167
    
    8

So "cleanedValues" is an instance variable of the class? You don't appear to be preventing two threads from accessing and modifying that variable in a bad way.

It would be better if it were local to the getValues method, and I don't see why it can't be.
Michael Melusky
Greenhorn

Joined: May 11, 2009
Posts: 5
My mistake, that ArrayList is instantiated locally within the method:

Paul Clapham
Bartender

Joined: Oct 14, 2005
Posts: 18167
    
    8

What is that synchronized(masterTable) block supposed to do? You don't use the map inside that block anywhere. All you're doing inside the synchronized block is copying the contents of one ArrayList into a second ArrayList in a complicated and probably unnecessary way.

So: first you get an ArrayList from the map. Then you copy its references into a second ArrayList for some reason. It's possible between those two steps something else could also get that same ArrayList from the map and do something to it.
Michael Melusky
Greenhorn

Joined: May 11, 2009
Posts: 5
In regards to the synchronized block around the iterator, I followed the instructions that were given from the Java doc:

http://java.sun.com/j2se/1.4.2/docs/api/java/util/Collections.html#synchronizedMap(java.util.Map)

We're using two ArrayLists, one to collect the whole set of data from the cache and the second to filter the data based on customization. I'm not sure whether to place a synchronized block around the get() method, since the class itself is already synchronized...
Paul Clapham
Bartender

Joined: Oct 14, 2005
Posts: 18167
    
    8

I'm asking why you synchronized a block on the Map object when you aren't using the Map in any way inside that block.
Michael Melusky
Greenhorn

Joined: May 11, 2009
Posts: 5
From the javadoc, Sun states that the synchronization block around the iterator is needed, even though the map itself isn't being accessed/mutated:

It is imperative that the user manually synchronize on the returned map when iterating over any of its collection views:

Map m = Collections.synchronizedMap(new HashMap());
...
Set s = m.keySet(); // Needn't be in synchronized block
...
synchronized(m) { // Synchronizing on m, not s!
Iterator i = s.iterator(); // Must be in synchronized block
while (i.hasNext())
foo(i.next());
}
Rob Spoor
Sheriff

Joined: Oct 27, 2005
Posts: 19557
    
  16

And you might want to check out the Collection.addAll; it makes your entire addValue method superfluous.


SCJP 1.4 - SCJP 6 - SCWCD 5 - OCEEJBD 6
How To Ask Questions How To Answer Questions
Paul Clapham
Bartender

Joined: Oct 14, 2005
Posts: 18167
    
    8

Michael Melusky wrote:From the javadoc, Sun states that the synchronization block around the iterator is needed, even though the map itself isn't being accessed/mutated...


Well, if the iterator were an iterator over the map, I would agree that would be a relevant (and important) piece of advice. But the iterator comes from an ArrayList, not from the map. So the map isn't involved in that synchronized block in any way whatsoever. And so that advice is not applicable.
Michael Melusky
Greenhorn

Joined: May 11, 2009
Posts: 5
In the code example from the javadoc, they're pulling the iterator from the Set of keys, not from the Map itself. I'm pulling the iterator from the List of values. In both cases, each object still references the same area of memory corresponding to the HashMap.
Paul Clapham
Bartender

Joined: Oct 14, 2005
Posts: 18167
    
    8

In the code you posted, you are NOT iterating over the values in the Map. First you get ONE of the values; it's an ArrayList. Then you get an iterator over the entries in that ArrayList.
 
I agree. Here's the link: http://aspose.com/file-tools
 
subject: Collections.synchronizedMap Help
 
Similar Threads
BST problem
FBNS: criteriaFind()
Cache Implementaion
Need Help for School assignment
Need Help for School assignment