File APIs for Java Developers
Manipulate DOC, XLS, PPT, PDF and many others from your application.
http://aspose.com/file-tools
Win a copy of Soft Skills: The software developer's life manual this week in the Jobs Discussion forum!
  • Post Reply
  • Bookmark Topic Watch Topic
  • New Topic

Collections.synchronizedMap Help

 
Michael Melusky
Greenhorn
Posts: 5
  • 0
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
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
Sheriff
Pie
Posts: 20156
23
MySQL Database
  • 0
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
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
Posts: 5
  • 0
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
My mistake, that ArrayList is instantiated locally within the method:

 
Paul Clapham
Sheriff
Pie
Posts: 20156
23
MySQL Database
  • 0
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
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
Posts: 5
  • 0
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
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
Sheriff
Pie
Posts: 20156
23
MySQL Database
  • 0
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
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
Posts: 5
  • 0
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
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
Pie
Posts: 20368
43
Chrome Eclipse IDE Java Windows
  • 0
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
And you might want to check out the Collection.addAll; it makes your entire addValue method superfluous.
 
Paul Clapham
Sheriff
Pie
Posts: 20156
23
MySQL Database
  • 0
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
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
Posts: 5
  • 0
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
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
Sheriff
Pie
Posts: 20156
23
MySQL Database
  • 0
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
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.
 
Don't get me started about those stupid light bulbs.
  • Post Reply
  • Bookmark Topic Watch Topic
  • New Topic