File APIs for Java Developers
Manipulate DOC, XLS, PPT, PDF and many others from your application.
http://aspose.com/file-tools
The moose likes Threads and Synchronization and the fly likes Possible read of stale data ? Big Moose Saloon
  Search | Java FAQ | Recent Topics | Flagged Topics | Hot Topics | Zero Replies
Register / Login
JavaRanch » Java Forums » Java » Threads and Synchronization
Bookmark "Possible read of stale data ?" Watch "Possible read of stale data ?" New topic
Author

Possible read of stale data ?

Kate Jackson
Greenhorn

Joined: Jan 12, 2010
Posts: 13
Hello,

Please consider following code snippet:



1. Is it possible that at LINE 1 I'd be reading stale data ? In this case the dispatchEvent method is not guaranteed to see the actual state of foos, right ? (Yes?)
2. Is it correct to assume that marking the iterablefoos as volatile at LINE 2 solves this problem ? (Yes?)
3. Do I even need to use the rc variable in getAllFoos() to make it work in original and with volatile ? (No?)

Thanks in advance,
Kate
Rakesh K. Cherukuri
Ranch Hand

Joined: Jun 01, 2010
Posts: 47

Quick question: Is there a reason foos.toArray() is used instead of foos.iterator() ?

Coming to your questions:

Assuming same instance (of the code you posted) is used by multiple threads

1. I think it is still possible. One scenario i see is this. Thread1 is trying to read and is at line 27 i.e rc has some value. At the same time Thread2 is adding/removing a value and set the iterablefoos to null. In this case Thread1 will see invalid value as iterablefoos is set to null by another thread.
2. Volatile will not solve the issue. Reason, the issue is not related to memory read/write. Problem is the iterablefoos is modified at multiple places and
- getAllFoos() is returning the value which could have been modified by some other thread.
- dispatchEvent() is not using synchronization
3. Using local variable rc is a good thing to make sure there is no reordering problems.

Assuming every thread gets its own instance
1. No. No, it is guaranteed. The synchronization in getAllFoos() makes sure of happens-before of the change in iterablefoos
2. No volatile use is required.
3. No. Since synchronization is in place rc is not required

Open to discussion though.

Warm Regards,
Rakesh
Kate Jackson
Greenhorn

Joined: Jan 12, 2010
Posts: 13
Thanks but what about that line 19 (LINE 1 comment) ?
We're trying to read the value of iterablefoos without synchronization and this reference is not volatile. Doesn't it mean that the reading thread might not see the null assigned by writing thread in add/removeFoo ? This would mean that apart from 1st read where the value is null initially we might not see any of the added/removed Foos, right ?
Henry Wong
author
Sheriff

Joined: Sep 28, 2004
Posts: 18117
    
  39

Kate Jackson wrote:Thanks but what about that line 19 (LINE 1 comment) ?
We're trying to read the value of iterablefoos without synchronization and this reference is not volatile. Doesn't it mean that the reading thread might not see the null assigned by writing thread in add/removeFoo ? This would mean that apart from 1st read where the value is null initially we might not see any of the added/removed Foos, right ?



I think that you need to define "stale" -- at what point is the data no longer valid.

Keep in mind that most systems don't cache with registers for many millis -- or at most a few hundred millis. The reason is that the system has other stuff to do, and hence, tend to flush the registers often. So, if the reference variable is not declared volatile, it won't get more than a few hundred millis before it will be synced with memory. If that is acceptable, then it won't ever get stale.

Second, just because something is volatile doesn't mean that it won't get stale. If your definition of stale is that it always need to be up to date, then what happens with the case where you load the reference, but before you use it (or finish using it), it gets changed. If that is your definition of stale, then volatile won't help -- you will need to grab the synchronize lock, and hold that lock until you finish processing the data.

Henry


Books: Java Threads, 3rd Edition, Jini in a Nutshell, and Java Gems (contributor)
Kate Jackson
Greenhorn

Joined: Jan 12, 2010
Posts: 13
Henry Wong wrote:
I think that you need to define "stale" -- at what point is the data no longer valid.


I must have gotten the wrong end of the stick, but isn't the LINE 1 read the same scenario as here:

If one thread calls setValue() there is no guarantee that subsequent reads in other threads via getValue() will ever see that value
(taken from java-concurrency-bugs-5-inconsistent-synchronization)

This would mean that at least theoretically there's a chance that the getAllFoos() method will never return actual state of the foos.
Henry Wong
author
Sheriff

Joined: Sep 28, 2004
Posts: 18117
    
  39

Kate Jackson wrote:I must have gotten the wrong end of the stick, but isn't the LINE 1 read the same scenario as here:

If one thread calls setValue() there is no guarantee that subsequent reads in other threads via getValue() will ever see that value
(taken from java-concurrency-bugs-5-inconsistent-synchronization)

This would mean that at least theoretically there's a chance that the getAllFoos() method will never return actual state of the foos.



It is generally a good idea -- when you read something and it makes no sense, that maybe it is wrong or the interpretation is wrong. After all, how can that make sense??? Where would getAllFoos() get this wrong values from???

What the statement is saying, since the get() method is not synchronized -- that it is possible for a later call to get() to catch up and pass the set() processing, and return an old value, before the set() call can finish finish. Think of it as the get() call running pass the set() call, and getting the previous value before the set call() can flush everything out to memory. It makes no sense that the get() call can return the previous value once everything is out to memory.

Henry

PS... the article in question is also talking about a bug triggered by this, which of course can happen -- but is a completely different issue.
Kate Jackson
Greenhorn

Joined: Jan 12, 2010
Posts: 13
Henry Wong wrote:
What the statement is saying, since the get() method is not synchronized -- that it is possible for a later call to get() to catch up and pass the set() processing, and return an old value, before the set() call can finish finish.


It really sounds convincing, but... correct me If I'm wrong but don't we need to enter synchronized block to even see the previous value written by the thread exiting from synchronized block ?
This is taken from jsr 133 faq (which I probably didn't get that's why I'm posting):

Synchronization ensures that memory writes by a thread before or during a synchronized block are made visible in a predictable manner to other threads which synchronize on the same monitor. After we exit a synchronized block, we release the monitor, which has the effect of flushing the cache to main memory, so that writes made by this thread can be visible to other threads. Before we can enter a synchronized block, we acquire the monitor, which has the effect of invalidating the local processor cache so that variables will be reloaded from main memory. We will then be able to see all of the writes made visible by the previous release.

Just to make it perfectly clear, here's the more simplistic sample:
My point is that we're trying to read the value of arr (String[] tmp = arr;) while not being in a synchronized block. Doesn't it mean that we are not guaranteed to see the updated value of the arr and I mean not even the previous value, just not see the update at all ?
The first time the getArray() was called the tmp = arr == null (we get the initial value as imposed by java memory model) but every other time since we're not in a synchronized block we might not see that the arr has been set to null and use the intial array instead, even if the add() method has been called a 1000 times.




Thanks again,
Rakesh K. Cherukuri
Ranch Hand

Joined: Jun 01, 2010
Posts: 47

Kate Jackson wrote:

I must have gotten the wrong end of the stick, but isn't the LINE 1 read the same scenario as here:

If one thread calls setValue() there is no guarantee that subsequent reads in other threads via getValue() will ever see that value
(taken from java-concurrency-bugs-5-inconsistent-synchronization)

This would mean that at least theoretically there's a chance that the getAllFoos() method will never return actual state of the foos.

I think there is one thing you are missing in your code. If you look closely, iterablefoos are read/modified within synchronized code (all 3 methods). The only exception is at line 19. There are two problems here
1. As you rightly said, you can see stale data which can be fixed by using volatile.
2. Even if you use volatile, as you are making a copy of the value while there is a chance that other threads can modify it by the time you use it in dispatchEvent(). Now As Henry rightly pointed out see if you agree that volatile is not going to solve the problem.
Henry Wong wrote:Second, just because something is volatile doesn't mean that it won't get stale. If your definition of stale is that it always need to be up to date, then what happens with the case where you load the reference, but before you use it (or finish using it), it gets changed. If that is your definition of stale, then volatile won't help -- you will need to grab the synchronize lock, and hold that lock until you finish processing the data.
Henry Wong
author
Sheriff

Joined: Sep 28, 2004
Posts: 18117
    
  39

Kate Jackson wrote:Synchronization ensures that memory writes by a thread before or during a synchronized block are made visible in a predictable manner to other threads which synchronize on the same monitor. After we exit a synchronized block, we release the monitor, which has the effect of flushing the cache to main memory, so that writes made by this thread can be visible to other threads. Before we can enter a synchronized block, we acquire the monitor, which has the effect of invalidating the local processor cache so that variables will be reloaded from main memory. We will then be able to see all of the writes made visible by the previous release.

Just to make it perfectly clear, here's the more simplistic sample:
My point is that we're trying to read the value of arr (String[] tmp = arr;) while not being in a synchronized block. Doesn't it mean that we are not guaranteed to see the updated value of the arr and I mean not even the previous value, just not see the update at all ?
The first time the getArray() was called the tmp = arr == null (we get the initial value as imposed by java memory model) but every other time since we're not in a synchronized block we might not see that the arr has been set to null and use the intial array instead, even if the add() method has been called a 1000 times.



Not sure of the point that you are trying to make.... if the point is, if there is a race condition, then it better be synchronized, then yes, I 100% whole heartily agree.

If the point is that the register caching optimization can cache an old value for more than 1000 method calls -- then I say "no way". Every implementation of the JVM syncs memory prior to a method call, and even if the JVM inlines the method (which elimiinates the method call setup), all current JVMs will sync prior to dealing with an interrupt. There is no way that 1000 calls can be done with a register cached value. In other words, just because there is a race condition, it doesn't mean that every possible case can happen.


To repeat, I 100% whole heartily agree that all race condition should be synchronized -- even though there may not be any bad affects..... However, there is no guarantee that a thread unsafe condition will have any bad affects. It could run perfectly every time forever. If there was a guarantee, then it would be much easier to debug thread race conditions.

Henry
Kate Jackson
Greenhorn

Joined: Jan 12, 2010
Posts: 13
Thanks that's just what I wanted to know.
 
I agree. Here's the link: http://aspose.com/file-tools
 
subject: Possible read of stale data ?
 
Similar Threads
Single Application but Multiple Connection problem
Explain the ouput
Double checked locking
Need a single instance of object.
Arrays of Objects