• Post Reply Bookmark Topic Watch Topic
  • New Topic

Checking for String value in this Collections.synchronizedSet(new HashSet()) ?  RSS feed

 
Amandeep Singh
Ranch Hand
Posts: 853
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
I have a code in which i am making a synchronized set.


Sun Documentation says-
It is imperative that the user manually synchronize on the returned set when iterating over it:



In my case, i am not iterating. I am checking for particular String in set using contains method, i do not have to use synchronized block.
like this-




Am i following Sun's documentation advice and the above code is safe without using the synchronized block?
 
Paul Clapham
Sheriff
Posts: 22834
43
Eclipse IDE Firefox Browser MySQL Database
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
The advice you posted is about iterating over the Set. You aren't iterating over the Set so the advice doesn't apply. Which is what you thought, correct?
 
Amandeep Singh
Ranch Hand
Posts: 853
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
1) Yes, the advice is about iterating. What i thought is- they just want to give a idea- it may apply to other scenario- when using the contains method.

2) If we are just checking through contains method without using a synchronized block. So what is the use of making a synchronizing a set? (Just to make sure, only 1 thread makes a set at a time.)



 
Adi Pandit
Greenhorn
Posts: 3
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
I believe you need to synchronize even if you are just calling setUPayerNums.contains(sPayerUnitNum).
Internally HashSet has a HashMap which goes through the entries in the relevant bucket to lookup the key passed. This means it is iterating a sub-set of the values. So if another thread is manipulating the Set you are not guaranteed the correct value and you could get some concurrency exceptions. I think the warning: "Failure to follow this advice may result in non-deterministic behavior." applies to the code you are trying.
 
Paul Clapham
Sheriff
Posts: 22834
43
Eclipse IDE Firefox Browser MySQL Database
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Adi Pandit wrote:I believe you need to synchronize even if you are just calling setUPayerNums.contains(sPayerUnitNum).
Internally HashSet has a HashMap which goes through the entries in the relevant bucket to lookup the key passed. This means it is iterating a sub-set of the values.


No, that's wrong. The advice quoted in the original post is about writing code which iterates over the set. It doesn't require the reader to know the internal implementation of other methods and to determine whether they might be iterating over something. It simply means what it says, no more than that.

If the contains() method actually does iterate over the set, or part of it, then the internal code will synchronize accordingly. That's precisely what the synchronizedSet method is for. All you have to worry about is code that you write. You don't have to second-guess code written by the Java API coders.
 
Henry Wong
author
Sheriff
Posts: 23295
125
C++ Chrome Eclipse IDE Firefox Browser Java jQuery Linux VI Editor Windows
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Amandeep Singh wrote:1) Yes, the advice is about iterating. What i thought is- they just want to give a idea- it may apply to other scenario- when using the contains method.


The set that is returned from the synchronizedSet() method returns a set that synchronizes all the method calls for you. The reason the iterator is a special case is because you don't want the set to change in-between many calls of the iterator, hence, you need to hold the lock to the set, for the full duration of the loop.

Amandeep Singh wrote:2) If we are just checking through contains method without using a synchronized block. So what is the use of making a synchronizing a set? (Just to make sure, only 1 thread makes a set at a time.)


Without using the synchronizedSet() method, then your set is not synchronized, and hence, you need to use the synchronized block for all the calls to the set. With the synchronizedSet(), then you only need to externally synchronize for the iterator, or any other operation that requires thread safety across mulitple calls to the set.

Henry
 
Adi Pandit
Greenhorn
Posts: 3
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
I guess my understanding was wrong. You do not need to synchronize when calling contains the wrapper class takes care of synchronizing on the internal lock being used when contains is invoked.

Code snippet from Collections.SynchronizedCollection




 
Amandeep Singh
Ranch Hand
Posts: 853
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Thanks Paul, Adi & Hengry.
 
G.Sathish kumar
Ranch Hand
Posts: 84
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
i feel locking the synchronised set object while iteration because access the right data like when my iteration there in 3 index so if i have new object add in 2 indexposition does give the issue.

on contains method checking also kind of iteration to check each data so i feel synchronizing the set object while contins method is the best.

please update me if i am wrong.
 
G.Sathish kumar
Ranch Hand
Posts: 84
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
sorry for the confusion just now i checked so contains methods internally synchronized so no need of locking
 
  • Post Reply Bookmark Topic Watch Topic
  • New Topic
Boost this thread!