aspose file tools*
The moose likes Threads and Synchronization and the fly likes Volatile is not working in the below code Big Moose Saloon
  Search | Java FAQ | Recent Topics | Flagged Topics | Hot Topics | Zero Replies
Register / Login
JavaRanch » Java Forums » Java » Threads and Synchronization
Bookmark "Volatile is not working in the below code" Watch "Volatile is not working in the below code" New topic
Author

Volatile is not working in the below code

Shaddy Khan
Greenhorn

Joined: Jul 26, 2013
Posts: 5



In the above code vloatile is not working...while if i make both the methods synchronized or count as AtomicInteger then it would work.
Please help me to figure out why volatile is not working......
Shaddy Khan
Greenhorn

Joined: Jul 26, 2013
Posts: 5


Modified version of above code. as i forget to mention that i m trying to make count as volatile.
please help me to figure out why vloatile is not working
Richard Tookey
Ranch Hand

Joined: Aug 27, 2012
Posts: 1057
    
  10

Shaddy Khan wrote:
please help me to figure out why vloatile is not working

What do you expect the output to be (and why) and what are you actually getting?
Steve Luke
Bartender

Joined: Jan 28, 2003
Posts: 4181
    
  21

Volatile is doing what it is supposed to do: it is acting as a happens-before barrier and making the changes to the value immediately visible to all threads. What it does not do is protect against inconsistent states or infer atomicity on code. Incrementing and decrementing values is a two-step process. If the process is interrupted then you will get inconsistent assignments. Volatile won't protect you from that.

If you want/need atomic actions like those in the code above you should use AtomicInteger, or if you have a large amount of competition for setting the value, or complex data to protect. you should use synchronization (Locks or synchronized blocks). But you can not use volatile in this case.


Steve
Winston Gutkowski
Bartender

Joined: Mar 17, 2011
Posts: 7784
    
  21

Steve Luke wrote:If you want/need atomic actions like those in the code above you should use AtomicInteger, or if you have a large amount of competition for setting the value, or complex data to protect. you should use synchronization (Locks or synchronized blocks). But you can not use volatile in this case.

@Shaddy Khan: I'm pretty sure (and I'm sure Steve et al will let me know if I'm wrong ) that once a variable (other than an array) is volatile, the only thing that needs to be synchronized is the action that writes to it.

So, in your case:would be all you need, and replace all instances of count++ with
setCount(count + 1);

However, if you're going to do that, why not just make it an AtomicInteger and let the class do all the heavy lifting?

Winston

[Edit: please read my next post as well]

Isn't it funny how there's always time and money enough to do it WRONG?
Articles by Winston can be found here
Winston Gutkowski
Bartender

Joined: Mar 17, 2011
Posts: 7784
    
  21

Winston Gutkowski wrote:I'm pretty sure (and I'm sure Steve et al will let me know if I'm wrong ) that once a variable (other than an array) is volatile, the only thing that needs to be synchronized is the action that writes to it.

Ooops. Just realized the flaw in my logic.

For Shaddy - and anyone else that's interested - the problem with my code above (I thought I'd leave it there as an example of faulty thinking) is that the evaluation of "count + 1" takes place in an unsynchronized area.

Thus, two threads could both read count with the same value, regardless of volatility, and thus both set it to the same "new value", when the intent was to increment it.

An "increment" - or indeed any "x = x + n" action - must be fully synchronized, regardless of the volatility of 'x'.

Hope that explains things; and also gives you some idea of just how tricky synchronization can be. As you can see: I still get it wrong; and I've been doing this stuff for quite a while.

So: don't use my code above - except as a cautionary tale, perhaps.

Winston
Shaddy Khan
Greenhorn

Joined: Jul 26, 2013
Posts: 5
@Winston and Steve: As per my knowledge about volatile, If you make any member as volatile in java it is loaded from main memory and if any thread changes its value then all threads are notified with its current value so if any other thread try to access the volatile member variable then that thread gets the current/latest value it contains.
If anything is wrong with my knowledge please let me know. I would be very thankful.

@ Richard: i expect the value of count would be 2000 each time i runs the program.
As per my code i m trying to increment the value of count to 2000 via two threads with help of volatile.

Ref# http://docs.oracle.com/javase/tutorial/essential/concurrency/atomic.html
Richard Tookey
Ranch Hand

Joined: Aug 27, 2012
Posts: 1057
    
  10

Shaddy Khan wrote:
@ Richard: i expect the value of count would be 2000 each time i runs the program.
As per my code i m trying to increment the value of count to 2000 via two threads with help of volatile.

Ref# http://docs.oracle.com/javase/tutorial/essential/concurrency/atomic.html


Unfortunately, being a read-modify-write, the ++ operator is not an atomic operation since a context switch can happen anywhere in the increment operation chain. I suspect that using 'green threads' (there used to be a command line switch but I think not available any more) that your code would have behaved as you expected since any context switch would have been controlled by the JVM but using native threads the JVM has lost control. I'm pretty sure you would have the same problem in C++ using 'ptheads' .

Using
one solves the problem but one still needs to make count 'volatile' because otherwise the JVM is at liberty to only update the value of 'count' when the increment() method exits.
Shaddy Khan
Greenhorn

Joined: Jul 26, 2013
Posts: 5
@Richard: i already made the count as volatile. As this code is testing of volatile only otherwise i can fix this case with the help of AtomicInteger or synchronization as mentioned in the my first post.
Richard Tookey
Ranch Hand

Joined: Aug 27, 2012
Posts: 1057
    
  10

Shaddy Khan wrote:@Richard: i already made the count as volatile. As this code is testing of volatile only otherwise i can fix this case with the help of AtomicInteger or synchronization as mentioned in the my first post.


You seem to have missed my point but never mind. Probably my bad English.
Steve Luke
Bartender

Joined: Jan 28, 2003
Posts: 4181
    
  21

Richard Tookey wrote:Using
one solves the problem but one still needs to make count 'volatile' because otherwise the JVM is at liberty to only update the value of 'count' when the increment() method exits.

I think the last sentence is incorrect, you do not need to use volatile in this case. The write to count will have a happens before relationship with the exit of the synchronized block, and the synchronized block will have a happens before relationship with the next synchronization. This is enough to make count visible to the other thread.

http://docs.oracle.com/javase/specs/jls/se7/html/jls-17.html#jls-17.4
17.4.3. Programs and Program Order
Among all the inter-thread actions performed by each thread t, the program order of t is a total order that reflects the order in which these actions would be performed according to the intra-thread semantics of t.

A set of actions is sequentially consistent if all actions occur in a total order (the execution order) that is consistent with program order, and furthermore, each read d of a variable v sees the value written by the write w to v such that:

* w comes before d in the execution order, and

* there is no other write w' such that w comes before w' and w' comes before d in the execution order.

17.4.4. Synchronization Order
Synchronization actions induce the synchronized-with relation on actions, defined as follows:

* An unlock action on monitor m synchronizes-with all subsequent lock actions on m

17.4.5. Happens-before Order
Two actions can be ordered by a happens-before relationship. If one action happens-before another, then the first is visible to and ordered before the second.

If we have two actions x and y, we write hb(x, y) to indicate that x happens-before y.

* If x and y are actions of the same thread and x comes before y in program order, then hb(x, y).

* If an action x synchronizes-with a following action y, then we also have hb(x, y).

* If hb(x, y) and hb(y, z), then hb(x, z).

A set of synchronization edges, S, is sufficient if it is the minimal set such that the transitive closure of S with the program order determines all of the happens-before edges in the execution. This set is unique.

It follows from the above definitions that:

* An unlock on a monitor happens-before every subsequent lock on that monitor.

* All actions in a thread happen-before any other thread successfully returns from a join() on that thread.


So we know that as long as the a read is sequentially consistent with the write, the read will see the write. We know that the unlock associated with the exit from a synchronized block infers synchronized-with other threads getting the synchronized lock. We also know that synchronizes-with infers sequential consistency (happens-before) as does programming order inside the same thread.

So, the write of count++ comes before the unlock of the synchronized block. We know the unlock comes before the lock of a subsequent thread. And we know the lock of the subsequent thread comes before the read of count in that thread. So we know the write in one thread comes before the read of the next thread, and will then be visible to that other thread. The only thing left is the print out at the end of the run, but that happens after a thread.join() and that also causes a synchronizes-with (and therefore happens-before) relationship (the last write happens before the end of the thread, the end of the thread happens before join() returns, and join() returns happens before reading count).

And we don't need volatile.
Steve Luke
Bartender

Joined: Jan 28, 2003
Posts: 4181
    
  21

Richard Tookey wrote:
Shaddy Khan wrote:@Richard: i already made the count as volatile. As this code is testing of volatile only otherwise i can fix this case with the help of AtomicInteger or synchronization as mentioned in the my first post.


You seem to have missed my point but never mind. Probably my bad English.


I think the point is worth repeating (third time at least):

You can not make compound statements atomic using the volatile keyword. That is not what it does. Incrementing is a a compound statement. So you can not make it atomic using volatile.

Yes, volatile makes a read see all writes from other threads. Yes, volatile makes all writes immediately visible to all threads. But a Read followed by a Write are two separate actions, and as such are able to be interrupted by another thread. An increment also has a Modify step between the Read and the Write steps providing more time for interruption. The sequence is this:

normal, single threaded case
Read: localCopy = count // 0
Modify: localCopy = localCopy + 1 // 1
Write: count = localCopy // 1

Read: localCopy = count // 1
Modify: localCopy = localCopy + 1 // 2
Write: count = localCopy // 2
...

possible, unsynchronized multi threaded case
T1: Read: localCopy = count // 0
T2: Read: localCopy = count // 0
T1: Modify: localCopy = localCopy + 1 // 1
T2: Modify: localCopy = localCopy + 1 // 1
T1: Write: copy = localCopy // 1
T2: Write: copy = localCopy // 1

Even though incrementing was done two times, since the steps were intermixed, the end result is only 1. Whats more is that this could happen:
T1: Read: localCopy = count // 0
T2: Read: localCopy = count // 0
T2: Modify: localCopy = localCopy + 1 // 1
T2: Write: copy = localCopy // 1
T2: Read: localCopy = count // 1
T2: Modify: localCopy = localCopy + 1 // 2
T2: Write: copy = localCopy // 2
T2: Read: localCopy = count // 2
T2: Modify: localCopy = localCopy + 1 // 3
T2: Write: copy = localCopy // 3
T1: Modify: localCopy = localCopy + 1 // 1
T1: Write: copy = localCopy // 1

Three increments happened, and T2 at one point gets the value up to 3. But because T2 does all its work between T1's read and write it is all undone. Volatile can't fix this.
Richard Tookey
Ranch Hand

Joined: Aug 27, 2012
Posts: 1057
    
  10

[quote=Steve Luke
I think the last sentence is incorrect, you do not need to use volatile in this case. .

Edit: You are right - shallow thinking on my part.
 
I agree. Here's the link: http://aspose.com/file-tools
 
subject: Volatile is not working in the below code