On 3/07/2010, at 3:03 PM, Adam Heath wrote: > 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; > } > } > ==
Your example is no different than the one I posted.
Are you trying to say that this won't work:
private static Map cache;
public static Object getObject(String key) {
if (cache.containsKey(key)) {
synchronized (cache) {
if (cache.containsKey(key) {
Object object = new Object();
cache.put(key, object);
}
}
}
return object;
}
I'm not saying you're wrong, I just want to get an example up of what I assumed
would work. Btw I never said I had read the book, just pieces.
>
> 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.
>
smime.p7s
Description: S/MIME cryptographic signature
