I want to add a Phobos module with template mixins for common idioms.

[Dmitry Olshansky]

20-May-2013 22:14, Idan Arye пишет:
> On Monday, 20 May 2013 at 16:40:27 UTC, Dmitry Olshansky wrote:
>> Long story short - re-read the discussion in the Low-lock thread again:
>> http://forum.dlang.org/thread/pelhvaxwjzhehdjtpsav@forum.dlang.org
>
> To sum up the discussion, there are three problems with unsynchronized
> access to the __gshared instance reference:
>
> 1) Unbaked object - the writer might write the __gshared reference
> before it finished the construction of the actual object.
>
> 2) Non-atomic read/write - this could result in a bad reference where
> the reader get some bytes from the old reference and some bytes from the
> new reference.
>
> 3) Unsynchronized cache - reader reads the reference, but the part of
> it's cache that is mapped to the memory containing the object instance
> itself is out of date and has not received the object yet.
>
> All these problems do not affect `hasInstance()` - which is the only

2 & 3 do.

> part of my implementation that touches the __gshared reference without
> synchronization - simply because it does not touch the object and does
> not return the reference - it only checks if it's initialized:

It _reads_ the _shared_ reference without proper indication of this 
intent to the writers.

>
> 1) It doesn't matter if the object is not ready, because when you want
> to actually access the object, you need to use `instance()` which has
> synchronization.

Then where you see hasInstance to fit the bill?

>
> 2) It does not matter if we get half a reference due to non-atomic
> read/write, because we only care if it's null or not. If the half
> reference we got is not null, that means the whole reference is not null
> and we have the correct answer.

Or you may never get the reference updated until the cache got flushed. 
It's generally not defined when you will see the update until then (or 
some such hardware event). The word is *eventually*.

> If the half reference we got is null -
> well, maybe the other half is not null, but the reference is only now
> being made not-null, so no harm is done it treating it as null for
> now(if we actually try to initialize it we will enter synchronization).

So it doesn't matter if hasInstance is fulfilling it's questionable 
contract properly only sometimes.

> 3) Since we don't try to access the object itself, we don't care that
> our local cache doesn't have it yet. Again - once we reach for the
> object itself, we will enter synchronization.

The big question is how you imagine somebody would want to use this

The false case may stay this way for unknown amount of time for instance 
even after the initialization happened (on some architectures). At the 
very least make that read atomicLoad that will make the operation 
properly tied to the current view of memory.

Even if we assume it's atomic then the other big question is what is the 
use case.
I argue that hasInstance only welcomes poor designs like:

while(!hasInstance()){
	//busy wait  for somebody else to init it?
}
inst = instance();

or:
if(hasInstance()){
	//should be initialized then the call won't construct it
	... //dunno what advantage it gets
}
else{
	//might or might not initialize/block on call to instance()
	...//again dunno
}


I'd say:

If you need to poke under it to avoid initialization then you shouldn't 
be using lazy-init singleton in the first place.

If you need synchronization and coordination based on what the reference 
happens to be right now then there are tools far better fit for the job 
- mutexes, semaphore, condition vars etc.

The last but not least is the fact that LowLock returns TLS reference to 
a (__g)shared instance make me worry about how the users code now is 
full of hidden race conditions anyway. This applies to the pattern as 
presented not only your implementation of it.
So the singleton itself would need some synchronization... and for that 
it really should be marked shared. The alternative is to have a 
per-thread singleton without any locking.

-- 
Dmitry Olshansky