google-code-export / google-guice

Automatically exported from code.google.com/p/google-guice
Apache License 2.0
2 stars 1 forks source link

document visibility failure in AbstractReferenceCache #89

Open GoogleCodeExporter opened 9 years ago

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

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

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