aspose file tools*
The moose likes Threads and Synchronization and the fly likes Bizarre synchronization Big Moose Saloon
  Search | Java FAQ | Recent Topics | Flagged Topics | Hot Topics | Zero Replies
Register / Login


Win a copy of Spring in Action this week in the Spring forum!
JavaRanch » Java Forums » Java » Threads and Synchronization
Bookmark "Bizarre synchronization" Watch "Bizarre synchronization" New topic
Author

Bizarre synchronization

Bojan Dolinar
Greenhorn

Joined: Nov 15, 2011
Posts: 6
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

Joined: Jan 03, 2004
Posts: 6109
    
    6

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
Sheriff

Joined: Sep 28, 2004
Posts: 18896
    
  40

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


Books: Java Threads, 3rd Edition, Jini in a Nutshell, and Java Gems (contributor)
Henry Wong
author
Sheriff

Joined: Sep 28, 2004
Posts: 18896
    
  40

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

Joined: Jan 03, 2004
Posts: 6109
    
    6

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

Joined: Nov 15, 2011
Posts: 6
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.

 
I agree. Here's the link: http://aspose.com/file-tools
 
subject: Bizarre synchronization