• Post Reply Bookmark Topic Watch Topic
  • New Topic

Synchronized Methods Returning Primitive Values  RSS feed

 
Jay Damon
Ranch Hand
Posts: 282
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Is it absolutely necessary to synchronize methods that return (read) primitive values, e.g. ints, longs, etc? I used to not think so but then I re-read the Java Tutorial on Synchronized Methods this morning and it seems to indicate that you should. I had always thought that, if I declared my setter methods synchronized thereby locking the object, I could avoid unnecessary synchronization of getter methods.

Specifically, I generally maintain a long value for the next refresh time in cached objects. My approach has always been to test this value first to see if a refresh is required and invoke the necessary synchronized methods only if true, thus avoiding unnecessary synchronization. I had always thought the worst case would be that some requestors would retrieve "slightly stale" data.

Am I wrong? Would someone care to clarify?

Also, when maintaining data in my objects, I generally use Hashtables and Vectors rather than HashMaps and ArrayLists which should alleviate the need to synchronize getter methods that return values from these collection objects. However, it now occurs to me that I should be synchronizing on these objects when clearing them and re-adding data. Otherwise, it would seem to me that there would be a brief period of time where these collection objects would have no data and could potentially return null, an incorrect result.

Am I correct? Should I specifically synchronize on collection objects when clearing and re-adding data?
 
Jim Yingst
Wanderer
Sheriff
Posts: 18671
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
[Jay]: Is it absolutely necessary to synchronize methods that return (read) primitive values, e.g. ints, longs, etc?

Absolutely? I suppose not, but it depends on what sort of errors you're willing to tolerate. My opinion is that many people go overboard avoiding synchronization, and create a lot of subtle errors that are difficult to reproduce or debug. So I would say, don't do it unless (a) you don't mind the occasional error, and (b) you can actually observe a performance difference between using synchronized code and unsynchronized code. Synchronization is much faster than it was in Java's early days, and much of the advice about avoiding synchronization is greatly exaggerated by today's standards.

[Jay]: I had always thought the worst case would be that some requestors would retrieve "slightly stale" data.

That's true for most primitives (if you're OK with stale data). Unfortunately for double and long, it's worse. Because Java generally uses 32-bit words under the covers, you can get strange effects for 64-bit datatypes. E.g. if you change 0x00000000 to 0xFFFFFFFF while another thread is reading the data, that other thread may see the value as 0x0000FFFF. This sort of word tearing can result in data which is not merely stale, but completely incorrect.

You can avoid this word tearing by declaring your long and double data as volatile. This also avoids the staleness issue. But you can still get some strange effects. A simple x++ may not work correctly, because it represents three distinct atomic actions:

If another thread changes x in between the first line and the third, the increment may be lost. So if two threads simultaneously increment the same variable, it may only get incremented once. The ways to fix that are either (a) use synchronization instead of volatile, or (b) use AtomicLong or similar classes from JDK 5.

Regarding Vector and Hashtable, yes, you should synchronize any time you perform two or more method calls and can't afford to let another thread access the collection in between calls. A common example of this is looping through a Vector:

What happens if an element is removed just as you get to the end? You could have a Vector of size 10. You set i = 9 and check that it's less than 10, good. Then another thread comes and removes an element, so the size is now nine. But then the first thread continues and tries to read element 9, resulting in an ArrayIndexOutOfBoundsException. Other scenarios allow for the possibility of skipping an element due to a remove, or processing an element twice due to an insert.

This is the primary reason why I dislike Vector and Hashtable, as well as the synchronized wrappers from methods like Collections.synchronizedList() etc. They tend to make the user think the code is thread-safe, when in reality they often require additional synchronization to be safe. I prefer to synchronize myself rather than relying on some unseen code that often synchronizes at the wrong level.
[ June 01, 2007: Message edited by: Jim Yingst ]
 
Jay Damon
Ranch Hand
Posts: 282
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Jim, Thanks for the outstanding response!

I agree with you: explicit synchronization is better. I will modify my code to just use HashMaps and ArrayLists and synchronize all getter methods as well as the methods to populate those collection objects. I suspect using Hashtables and Vectors (both declared final) as I was with my getter methods is okay as I was only invoking a single collection method but, by using explicit synchronization, the code will be safer and better documented for the next developer.

Thanks also for the long/double/volatile explanation.
 
  • Post Reply Bookmark Topic Watch Topic
  • New Topic
Boost this thread!