• Post Reply Bookmark Topic Watch Topic
  • New Topic
programming forums Java Mobile Certification Databases Caching Books Engineering Micro Controllers OS Languages Paradigms IDEs Build Tools Frameworks Application Servers Open Source This Site Careers Other Pie Elite all forums
this forum made possible by our volunteer staff, including ...
Marshals:
  • Campbell Ritchie
  • Jeanne Boyarsky
  • Ron McLeod
  • Paul Clapham
  • Liutauras Vilda
Sheriffs:
  • paul wheaton
  • Rob Spoor
  • Devaka Cooray
Saloon Keepers:
  • Stephan van Hulst
  • Tim Holloway
  • Carey Brown
  • Frits Walraven
  • Tim Moores
Bartenders:
  • Mikalai Zaikin

Collections.synchronizedMap Help

 
Greenhorn
Posts: 5
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • 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.

 
Marshal
Posts: 28193
95
Eclipse IDE Firefox Browser MySQL Database
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • 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
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
My mistake, that ArrayList is instantiated locally within the method:

 
Paul Clapham
Marshal
Posts: 28193
95
Eclipse IDE Firefox Browser MySQL Database
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • 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
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • 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
Marshal
Posts: 28193
95
Eclipse IDE Firefox Browser MySQL Database
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • 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
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • 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());
}

 
Sheriff
Posts: 22783
131
Eclipse IDE Spring VI Editor Chrome Java Windows
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
And you might want to check out the Collection.addAll; it makes your entire addValue method superfluous.
 
Paul Clapham
Marshal
Posts: 28193
95
Eclipse IDE Firefox Browser MySQL Database
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • 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
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • 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
Marshal
Posts: 28193
95
Eclipse IDE Firefox Browser MySQL Database
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • 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.
 
reply
    Bookmark Topic Watch Topic
  • New Topic