• Post Reply Bookmark Topic Watch Topic
  • New Topic

what's the side effect of mutator not synchonized?  RSS feed

 
David Spades
Ranch Hand
Posts: 348
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator


what's the side effect of this code? I intentionally didn't mark throwOut as synchronized and I'm pretty sure that I read somewhere that this is discouraged, but I couldn't think of any side effect. Can anybody give an example where this code would lead to problem?
thanks
 
Tim Cooke
Marshal
Posts: 3652
184
Clojure IntelliJ IDE Java
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
David Spades wrote:what's the side effect of this code?

Do you have any thoughts on it? Let's hear them first.
 
Henry Wong
author
Sheriff
Posts: 22861
119
C++ Chrome Eclipse IDE Firefox Browser Java jQuery Linux VI Editor Windows
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator

Also, what was the reason that you synchronized the add() method? Does that same reason apply to the remove method?

Henry
 
David Spades
Ranch Hand
Posts: 348
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator


amended the code, sorry, it's not remove. It should've been size() method. IMO, getSize() doesn;t need synchronizing. lets say another thread is in the middle of adding a new element when getSize is called, so the new element is not yet included in the count, which is valid since the adding is not yet finished, right?
this is my reasoning. is there any fault in this?
thanks
 
Henry Wong
author
Sheriff
Posts: 22861
119
C++ Chrome Eclipse IDE Firefox Browser Java jQuery Linux VI Editor Windows
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
David Spades wrote: IMO, getSize() doesn;t need synchronizing. lets say another thread is in the middle of adding a new element when getSize is called, so the new element is not yet included in the count, which is valid since the adding is not yet finished, right?
this is my reasoning. is there any fault in this?


Can you show us where in the JavaDoc that the implementation is to be as you described? And to follow that, show us that the implementation is true for all versions, and future versions, of the JVMs?

Your reasoning, while sound, is still an assumption of the implementation. You need to code based on specification/documentation, and not based on a particular implementation (although admittedly, you can only test via implementation).

Henry
 
Chris Hurst
Ranch Hand
Posts: 443
3
C++ Eclipse IDE Java
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
David Spades wrote: IMO, getSize() doesn;t need synchronizing. lets say another thread is in the middle of adding a new element when getSize is called, so the new element is not yet included in the count, which is valid since the adding is not yet finished, right?
this is my reasoning. is there any fault in this?


getSize needs to be synchronized. If one thread does all the adds and another was created at the same time as the first but never synchronizes on a common object with the mutating thread, getSize could in theory always return 0 for the second thread always/for ever i.e. empty collection regardless of any modifications by other threads because it never looks for change, the answer was zero the last time so surely its still zero as this thread hasn't modified it. By not not saying getSize is synchronized you are effectively allowing the reading thread to ignore all other threads changes if it decides its more efficient to do so i.e. your saying you don't care but you obviously do. In reality your getSize will most likely be observed to work (ish) but why take the risk ??
 
Anupam Sinha
Ranch Hand
Posts: 1090
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Chris Hurst wrote:
David Spades wrote: IMO, getSize() doesn;t need synchronizing. lets say another thread is in the middle of adding a new element when getSize is called, so the new element is not yet included in the count, which is valid since the adding is not yet finished, right?
this is my reasoning. is there any fault in this?


getSize needs to be synchronized. If one thread does all the adds and another was created at the same time as the first but never synchronizes on a common object with the mutating thread, getSize could in theory always return 0 for the second thread always/for ever i.e. empty collection regardless of any modifications by other threads because it never looks for change, the answer was zero the last time so surely its still zero as this thread hasn't modified it. By not not saying getSize is synchronized you are effectively allowing the reading thread to ignore all other threads changes if it decides its more efficient to do so i.e. your saying you don't care but you obviously do. In reality your getSize will most likely be observed to work (ish) but why take the risk ??



Can getSize always return 0 for the current code? Doesn't synchronize also provides a happens before guarantee?
 
Chris Hurst
Ranch Hand
Posts: 443
3
C++ Eclipse IDE Java
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Anupam Sinha wrote:Can getSize always return 0 for the current code? Doesn't synchronize also provides a happens before guarantee?


If two threads synchronize on the same object you have your guarantee, so if two threads both 'add' it works but if another thread calls just getSize looking for objects to work with say it potentially doesn't :-( ie another thread could just call getSize and never sync at all.
In some edge cases the synchronized key word can be optimized away by Java i.e. if it can tell no other thread will ever contend, it was a common misconception that adding synchonized guarantees a memory barrier.

The add method has a read/write barrier but getSize has none at all so you have to work out the last sequence point for the two threads. The thread calling getSize may have effectively cached the size of the collection at zero in theory forever however changing getSize to synchronized prohibits any compiler optimization, caching etc etc i.e. its a volatile read the add method being a volatile write on its own may not be enough.
 
Don't get me started about those stupid light bulbs.
  • Post Reply Bookmark Topic Watch Topic
  • New Topic
Boost this thread!