Scott Gray wrote:
> On 3/07/2010, at 2:13 PM, Adrian Crum wrote:
>
>> --- On Fri, 7/2/10, David E Jones <[email protected]> wrote:
>>> On Jul 2, 2010, at 7:11 PM, Adrian Crum wrote:
>>>
>>>> Because I've done it myself. ;-)
>>> Wait, do you mean that in this case you did the actual
>>> copying and pasting? Well... I guess that would be one way
>>> to know for sure!
>> Btw, I'm not implying the code Scott is working on was copy-and-paste code -
>> I was just making a generalized statement about where my head is at lately.
>>
>> When I first got involved with Java, I would copy and paste
>> double-checked-locking code blocks because I assumed they were necessary. I
>> didn't bother to research why they were there. If I had, I would have
>> learned that they don't work and there are better ways to do
>> synchronization. That is only one example of my uninformed copy-and-paste
>> code development - there are more.
>
> This has always bothered me, as far as I can tell double-checked locking DOES
> work just fine, the only time it doesn't is when you do the following:
> private static Object object;
>
> public static Object getObject() {
> if (object == null) {
> synchronized (Some.class) {
> if (object == null) {
> object = new Object();
> }
> }
> }
> // potentially not fully initialized!!
> return object;
> }
>
> But we very rarely use the pattern in that manner. What we do usually is
> initialize the object and then in an additional statement we assign it to the
> variable being checked. The potential for returning partially initialized
> objects is the concurrency book's biggest argument against the pattern I
> don't often see that potential in the ways that OFBiz uses it.
>
> Regards
> Scott
==
if (object = null) {
synchronized(lock) {
if (object == null) {
Object foo = getFoo();
Object bar = getBar();
object = new Result(foo, bar);
}
}
}
return object;
class Result {
Object foo, bar;
Result(Object foo, Object bar) {
this.foo = foo;
this.bar = bar;
}
}
==
This is buggy too, and is a much more common pattern. In fact, this
is the only argument needed to remove all DCL patterns.
When a new object is created, a memory block is allocated. The
address of that block is then stored in object. The memory block is
then initialized. The block is not fully initialized until the
constructor is returned from.
After the address of the block is stored in the object, then the outer
conditional thinks the object is no longer null. This is true. But
!= null does not mean the object has been initialized yet.
If you try to assign the new object to a tmp first, then assign to the
final output, it won't help, because the memory assignment could still
be reordered. All memory read/writes that occur inside a synchronized
block can be reordered in any fashion. One of those memory accesses
is the storage of the allocated objects memory address into the
unprotected object. If the memory access can be reordered, then it's
possible that the instances variable assignments could happen after
the object assignment, which then means the returning of a non-null,
uninitialized object.
Yes, it's complex. If you don't understand my ramblings(the larger
paragraph), then you need to go back and re-read the concurrency book.
