File APIs for Java Developers
Manipulate DOC, XLS, PPT, PDF and many others from your application.
http://aspose.com/file-tools
  • Post Reply
  • Bookmark Topic Watch Topic
  • New Topic

Bizarre synchronization

 
Bojan Dolinar
Greenhorn
Posts: 6
  • 0
  • Mark post as helpful
  • send pies
  • 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.
 
Jeff Verdegan
Bartender
Posts: 6109
6
Android IntelliJ IDE Java
  • 0
  • Mark post as helpful
  • send pies
  • 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.


 
Henry Wong
author
Marshal
Pie
Posts: 20826
75
C++ Chrome Eclipse IDE Firefox Browser Java jQuery Linux VI Editor Windows
  • 0
  • Mark post as helpful
  • send pies
  • 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
Marshal
Pie
Posts: 20826
75
C++ Chrome Eclipse IDE Firefox Browser Java jQuery Linux VI Editor Windows
  • 0
  • Mark post as helpful
  • send pies
  • 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
  • 0
  • Mark post as helpful
  • send pies
  • 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
  • 0
  • Mark post as helpful
  • send pies
  • 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.

 
Don't get me started about those stupid light bulbs.
  • Post Reply
  • Bookmark Topic Watch Topic
  • New Topic