• 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

Bizarre synchronization

 
Greenhorn
Posts: 6
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
I was checking some old code of ours. It is multithreaded, but the author commited some beginner mistakes, for example not synchronizing booleans and so. But this piece of code might even work, but it seems bizarre to me:



Each of these methods locks object and in the synch block it assigns new object to the reference. Both integers are carrying two roles now: semantics and synchronization.
However, after some head scratching I figured that it should work. I wrote test and it passes. I'm still inclined to change the code since it looks more like a contrived example for programming exam as it just makes you wonder. This can't be an example of idiomatic MT programming, right? If not, I will just use AtomicInteger, probably.
 
Bartender
Posts: 6109
6
Android IntelliJ IDE Java
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator

Bojan Dolinar wrote:Both integers are carrying two roles now: semantics and synchronization.



That's not really a problem. In fact, we do it all the time. Every time you declare a method synchronized, the "this" object is carrying both roles. And it happens every time we use Collections.synchronizedXxx, or a Vector, Hashtable, or StringBuffer. I can't say I've ever heard of doing it on an immutable object though. Was that your point?

However, after some head scratching I figured that it should work. I wrote test and it passes.



It won't work. And it's near impossible to test multithreaded code. It might work a million times in a row but fail on the milliona-and-first.

It won't work because the following can happen:


In other words, synchronized (X) is not atomic. We first have to read the value of X, then we have to obtain the lock on the object it points to. (I've kind of mixed references and objects in my example. I hope it's still clear.)

We've now had 2 increments, but X has only increased by 1.

So if both T1 and T2 read the value of X before either obtains the lock, and then T1 obtains the lock on that object, then changes X to point to a different object, that doesn't change the fact that T2 has already read the reference for the first object, and can lock it after T1 is done, and will never know that T1 has now said that we're actually using a different object.

If I were you, I'd change all those to AtomicIntegers.


 
author
Posts: 23951
142
jQuery Eclipse IDE Firefox Browser VI Editor C++ Chrome Java Linux Windows
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator

Bojan Dolinar wrote:I was checking some old code of ours. It is multithreaded, but the author commited some beginner mistakes, for example not synchronizing booleans and so. But this piece of code might even work, but it seems bizarre to me:



Each of these methods locks object and in the synch block it assigns new object to the reference. Both integers are carrying two roles now: semantics and synchronization.
However, after some head scratching I figured that it should work. I wrote test and it passes. I'm still inclined to change the code since it looks more like a contrived example for programming exam as it just makes you wonder. This can't be an example of idiomatic MT programming, right? If not, I will just use AtomicInteger, probably.



Your gut feeling is correct. Anytime you change the lock object while it is being used for synchronization, it means that it is possible to synchronize on two different objects.

[edit: looks like Jeff beat me, and has the more detailed answer.]


Henry
 
Henry Wong
author
Posts: 23951
142
jQuery Eclipse IDE Firefox Browser VI Editor C++ Chrome Java Linux Windows
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator

Jeff Verdegan wrote:
It won't work because the following can happen:



Side note. A lock grab causes a flush of the caches, so it is unlikely that X won't be reread after the lock grab. However, this can happen...



Henry
 
Jeff Verdegan
Bartender
Posts: 6109
6
Android IntelliJ IDE Java
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator

Henry Wong wrote:

Jeff Verdegan wrote:
It won't work because the following can happen:



Side note. A lock grab causes a flush of the caches, so it is unlikely that X won't be reread after the lock grab.



I think that caches are flushed after the lock is already obtained. That is, once we've got the lock, the first read of every variable must come from main mem. So, T2 read's X, gets the reference to the 1 Object, then T1 does its update, then T2 obtains the lock for the object pointed to by the value it has already read.

Also note that neither my example nor yours actually require the sequential operation shown. Another possibility is that T1, T2, ..., TN are all on separate processors, and all read X at the same time. All will try to obtain the lock for that object, one will win, and the others will wait. Once the lock is release, the waiting threads don't re-read the variable and switch their attempt to the new object--they continue to each obtain the lock for the original object in turn.

The main point, of course, is don't change the shared reference variable you used for syncing while you're holing its lock. (Which really means don't change it ever, since you can't know when other threads may have already read the value in preparation for syncing.)
 
Bojan Dolinar
Greenhorn
Posts: 6
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator

Jeff Verdegan wrote:I can't say I've ever heard of doing it on an immutable object though. Was that your point?



Exactly! Due to immutability you have to assign new object to reference, which defines the problem.

I was not aware of the fact that synchronized(X) is not atomic (I had to examine the context to find out whether this was a statement or just a situational consequence of your example). That was the reason I incorrectly decided code was still ok. Your explanation was great. Thank you for your time.

 
reply
    Bookmark Topic Watch Topic
  • New Topic