*
The moose likes Threads and Synchronization and the fly likes Need help understanding how to synchronize methods correctly Big Moose Saloon
  Search | Java FAQ | Recent Topics | Flagged Topics | Hot Topics | Zero Replies
Register / Login
JavaRanch » Java Forums » Java » Threads and Synchronization
Bookmark "Need help understanding how to synchronize methods correctly" Watch "Need help understanding how to synchronize methods correctly" New topic
Author

Need help understanding how to synchronize methods correctly

Sean Doherty
Greenhorn

Joined: Apr 29, 2009
Posts: 13
I have the following scenario:

There is a singleton class which maintains a cache which is used concurrently by multiple threads. Below is a code snippet
for the singleton class with the cache.

class Singleton {

private static final Singleton m_instance = new Singleton();

private Singleton() {}

public static Singleton getInstance() {
return m_instance;
}


private Map<Integer, List<Object>> m_cache = new ConcurrentHashMap<Integer, List<Object>>();

/**
* Assume here that the cache is initialized and the gets do have values for
* simplicity.
*/
public List<Object> get(Integer key) {
return m_cache.get(key);
}

public void put(Integer key, List<Object> value) {
m_cache.put(key, value)
}
}



/**
* Multiple instances of this class will be running at the same time.
*/
Class Consumer {

public void execute() {

// Get the list from the cache.
List<Object> list = Singleton.get(10);

/*
* Read the data from the list read from the cache in a loop and create a list map.
*/
List<Object> list2; // This list is a new list created, initialized and populated here.

// Write into the cache.
Singleton.getInstance().put(10, list2);
}
}

Now the question is:
Even though the get method of the Singleton class is safe due to the underlying ConcurrentHashMap get, we spent some time reading the list.
How do we handle the case where the list is updated by another thread during the read?

The thing is after the list is read, i don't care as the list will be replaced by a newly generated list.


Any comments, suggestions on the topic will be greatly appreciated.
Paul Clapham
Bartender

Joined: Oct 14, 2005
Posts: 18541
    
    8

"Neon", please check your private messages regarding an important administrative matter.

Thank you.
Sean Doherty
Greenhorn

Joined: Apr 29, 2009
Posts: 13
Can any of the concurrency gurus please reply to my post :-)
Steve Luke
Bartender

Joined: Jan 28, 2003
Posts: 4181
    
  21

Sean Doherty wrote:Can any of the concurrency gurus please reply to my post :-)


Well, the color for your code makes it real hard to read. So hard, I give up trying. Maybe go back and edit your post, remove the colors, and use code tags instead...

So ignoring your code you seem to be asking about a good way to synchronize your maps. Using a Copy-On-Write approach is a pretty good way to keep things thread safe. But it does cost resources. So if you have a lot of Write operations, this may be prohibitive.

Other approaches would be a synchronized scheme. Perhaps instead of passing around an instance of a Map with Lists in them, you provide an interface. People use an interface to add or retrieve a value to the Cache. The cache synchronizes internally, never releasing either the map, or the List to the wild were clients can try to break it. If the client needs to retrieve a complete list from the Cache, then you make a deep copy of the list and release it. If they need to set a complete list to the Cache, you make a deep copy and store it (or add it to the already present list). This prevents you from having to deep copy an entire Map at each Write.


Steve
Sean Doherty
Greenhorn

Joined: Apr 29, 2009
Posts: 13
Sorry about that coz i am viewing it through firefox browser and it looks quite fine to me. Anyway, i have edited the code to be in black print. Also i have changed the code to be simpler. Any comments greatly appreciated.
Steve Luke
Bartender

Joined: Jan 28, 2003
Posts: 4181
    
  21

Now the scenario is a quite different, I think...

"Now the question is:
Even though the get method of the Singleton class is safe due to the underlying ConcurrentHashMap get, we spent some time reading the list.
How do we handle the case where the list is updated by another thread during the read?

The thing is after the list is read, i don't care as the list will be replaced by a newly generated list."

How is the list you get out of the Map related to the list that gets put in? How could another thread Modify the list you pull out while you read?

If all you need to do is store the List that one thread puts into the cache so that some other thread can read it, without worrying about another thread modifying it, I would suggest having your put() method generate a copy of the incoming list. If the list will be read-only after it was read, I would suggest wrapping it (the copy not the original) in an unmodifiableList (see java.util.Collections class) and storing/returning that unmodifiable view.




If the List needs to editable in the Consumers that retrieve them, then what you need to do depends on how you want the different threads to react. If you want two Threads reading/modifying the same List to be able to see each others changes, then you should use a Thread Safe List to store in your Map, like the CopyOnWriteArrayList. There are other ways of making the List thread safe but they tend to require the Consumer to make sure the synchronize their read/write/iteration.




If the lists need to be editable, but one thread should not see the edits from another thread, then copying the List (again) on the way out of the get() method is the way to go.



Sean Doherty
Greenhorn

Joined: Apr 29, 2009
Posts: 13
Hey Steve,
Thanks a lot for the elaborate and detailed explanation. Well i understand what you are trying to convey.

Ok here's the deal, the cache stores an Id and with each Id is associated a list of objects. Multiple threads read and write into the map. At any point of time, any thread can read the list for the given id, then generate a new list and replace the one in the map. Ideally speaking each thread should have a different set of id's to work with at a given instance of time. With that said, each write should ideally not affect the other.

Mentioning that, i would assume that i do nothing and it should be fine. As my gets are safe (thanks to ConcurrentHashMap), and my puts are safe too. So i think just replacing the Map with the ConcurrentHashMap should free me of all the synchronized blocks or methods.

Also the idea of using Collections.unmodifiableList sounds good, but i am assuming that i don't require it as on every write, the list is replaced.
Please correct me if i am wrong....
Steve Luke
Bartender

Joined: Jan 28, 2003
Posts: 4181
    
  21

It depends on how well you trust everyone who will use your application.

" Ideally speaking each thread should have a different set of id's to work with at a given instance of time. With that said, each write should ideally not affect the other. "

But if some accident, miscalculation, or unforeseen circumstance occurs then a single list could be in use by multiple consumers. You should protect against this if you can. And in this case you can rather easily.

Then again, there are always those instances where the client code (the code using this cache) is deliberately malignant and can use un-protected caches to break something. Again, something you can protect against by properly securing your values.

" i would assume that i do nothing and it should be fine. As my gets are safe (thanks to ConcurrentHashMap), and my puts are safe too"

But the List still isn't. You can assume that when the client code is done with making a list, it uses put() to give the List to the cache then immediately stops using it. But that may not be the case. What if some coder decides that a more efficient means of operation is to re-use the list over and over again? Or to submit the results in chunks? Or what if the List was accidentally leaked to some other code which then keeps it around, reads/writes from it, etc... Without protecting the Lists, you might be safe. But you might not be, and if there is a problem it could be catastrophic and hard to discover.

"Also the idea of using Collections.unmodifiableList sounds good, but i am assuming that i don't require it as on every write, the list is replaced.
Please correct me if i am wrong...."

I like to play it safe. If you copy coming in and going out, you will be safe. If you don't, you might be safe as long as everyone who touches the code knows the rules which have to be followed to keep it safe and accidents aren't made.
Sean Doherty
Greenhorn

Joined: Apr 29, 2009
Posts: 13
Appreciate your comments Steve. Yes will definitely consider the things you have mentioned. Thanks again for the information sharing.
 
I agree. Here's the link: http://aspose.com/file-tools
 
subject: Need help understanding how to synchronize methods correctly