just to clarify - the volatile in that link referred to C language 'volatile' keyword that happened to be used in the JDK native implementation of park(), unpark() routines. C language 'volatile' only prevents compiler optimizations - it doesn't apply memory barriers after/before load/store.
i remember we had a terrible lost interrupt in hdfs once that was similar to this problem (as ryan finally described it). On Thu, Apr 15, 2010 at 3:28 PM, Kannan Muthukkaruppan <kan...@facebook.com> wrote: > Ryan: > > Joy had brought up a related issue to attention recently, and fwd'ed this > link: > > http://blogs.sun.com/dave/entry/a_race_in_locksupport_park > > It has to do with a volatile "_counter" variable as well. > > From the above writeup: > > <<< The problem would only manifest when we were using the -UseMembar > optimization that lets us remove fences from certain hot thread state > transitions paths that need to coordinate safepoints between mutator threads > and the JVM. This feature is enabled by default, but we can turn it off with > the -XX:+UseMembar switch, which causes the JVM to emit normal fence > instructions in the state transitions paths. (That particular optimization is > an example of asymmetric Dekker synchronization). Crucially, the park() path > contains such a state transition. In reality the fence emitted by the > +UseMembar switch was simply covering up the otherwise latent Parker:: bug. > +UseMembar constitutes a work-around. Sensitivity to UseMembar was initially > confounding but ultimately a valuable clue.>>> > > > -----Original Message----- > From: Dhruba Borthakur [mailto:dhr...@gmail.com] > Sent: Thursday, April 15, 2010 3:24 PM > To: hbase-dev@hadoop.apache.org > Subject: Re: volatile considered harmful > > I agree. The Java spec clearly mentions that load/store/read/write to > volatile are atomic > > http://java.sun.com/docs/books/jls/second_edition/html/memory.doc.html#28330 > > <http://java.sun.com/docs/books/jls/second_edition/html/memory.doc.html#28330>but > definitely not increment and decrement. increment and decrement operations > need to load *and* store data to a volatile variable and are not atomic. > > -dhruba > > > On Thu, Apr 15, 2010 at 3:00 PM, Ryan Rawson <ryano...@gmail.com> wrote: > >> So there I was, running a unit test. I step away and hours later it >> still had not finished. It was stuck on this line of the code: >> >> putThread.done(); >> > putThread.join(); >> >> So the putThread was not finishing as it was supposed to do. Well, >> what does putThread.done() do: >> >> public void done() { >> done = true; >> synchronized (this) { >> interrupt(); >> } >> } >> >> >> Looks reasonable, wait, what is this access of a variable shared among >> multiple threads outside of a sync block? Oh wait, its "ok", it's >> volatile: >> >> private volatile boolean done; >> >> >> And what does the run loop of the thread look like? Again reasonable: >> >> public void run() { >> done.set(false); >> int val = 0; >> while (!done.get()) { >> // Do HRegion.put call with some exception handling >> } >> } >> >> >> Well in theory this all looks reasonable. >> >> But why harmful? >> >> I had changed the Gets to not acquire row locks, thus removing a >> synchronization block between the main thread (which did Gets and then >> eventually called that code up top). Two consequences, one was puts >> happened much faster (good!), and a negative consequence is sometimes >> the test would fail to stop the put thread. For minutes and hours >> after the main thread had signalled the put thread to stop, it still >> had not. >> >> I switched the volatile boolean => AtomicBoolean and the problem went away. >> >> In short, the use of volatile is not guaranteed to see an update from >> a different thread in any timely manner. The Java Memory Model does >> not ensure this. If you need to rely on passing data between threads >> you need to either use a monitor lock (using synchronized keyword >> either at the method level or as an explicit block) or use one of the >> concurrent utilities classes - such as AtomicLong, AtomicBoolean, or >> the excellent read/write locks. >> >> If you wish to increment a counter from multiple threads, doing this: >> volatile int count = 0; >> // later: >> count++; >> >> will not work. You must use an AtomicInt/Long or use a monitor lock. >> This is because the operation "count++" is not atomic, it is actually >> 3 - get, add, put. >> >> As we code in HBase, we should avoid the use of volatile as much as >> possible. The guarantees it makes are not good enough and can cause a >> lot of pain figuring this out months after you originally coded it. >> > > > > -- > Connect to me at http://www.facebook.com/dhruba > >
