document visibility failure in AbstractReferenceCache

GoogleCodeExporter commented 9 years ago

The private boolean field "set" of class FutureValue (nested in
AbstractReferenceCache) is accessed without synchronization in
FutureValue.get(). There is no happens-before edge between setting the
field's value to true and reading the value.

The code isn't really broken, because the worst that can happen is that
waitUntilSet() is called unnecessarily. So the test of (!set) is really an
optimization that is only likely to win in certain situations, i.e., single
CPUs. But in those systems the cost of a mostly uncontended lock
acquisition is negligible.

If you have performance data that suggests that it really is a win,
document it. Otherwise I suggest taking it out -- it's just confusing.

Original issue reported on code.google.com by tpeie...@gmail.com on 10 Apr 2007 at 4:39

GoogleCodeExporter commented 9 years ago

Original comment by kevin...@gmail.com on 20 Apr 2007 at 2:52

Added labels: 1.1

GoogleCodeExporter commented 9 years ago

Hmmmm... I think I meant for get() to be synchronized, i.e. this wasn't 
intentional. Eek.

Original comment by crazybob...@gmail.com on 11 Sep 2007 at 1:04

GoogleCodeExporter commented 9 years ago

Original comment by limpbizkit on 30 May 2008 at 6:56

Added labels: Milestone-Release2.0
Removed labels: 1.1

GoogleCodeExporter commented 9 years ago

This was fixed a while back in r374. From mcculls:
> using DCL could make things a bit faster, but if performance
> is ok with the current code then I'd say issue 89 is also fixed

Original comment by limpbizkit on 16 Jul 2008 at 3:43

Changed state: Fixed

pombreda / google-guice

document visibility failure in AbstractReferenceCache #89