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

Idan Arye GenericNPC at gmail.com
Mon May 20 15:02:56 PDT 2013


On Monday, 20 May 2013 at 19:15:34 UTC, Dmitry Olshansky wrote:
> 20-May-2013 22:14, Idan Arye пишет:
>> 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?

`hasInstance()` tells you if an instance is already initialized, 
without returning it and without risking in initializing it if it 
isn't already initialized.

>> 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*.

When you call `instance()` to fetch the reference(opposed to 
`hasInstance()`, which only tells you if it's null), the thread 
will enter the synchronization block inside `instance()` which 
will make sure the cache is refreshed before it begins.

>> 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.

If `hasInstance()` returns `true`, you can assume that there is 
an instance for you to access, because even if the instance is 
not ready yet, some other thread has entered the synchronization 
block and the user can't get the instance until that thread 
finishes the instantiation - so from the thread's point of view, 
whenever it'll call `instance()` it'll get a ready instance that 
was not created beacause of it.

If `hasInstance()` returns `false` then it's not a guarantee that 
the singleton has not been instantiated, but even if 
`hasInstance()` was synchronized you wouldn't get that guarantee, 
because it is always possible that the singleton is instantiated 
after the synchronization in `hasInstance()` but before you get 
to act upon it's result.

The only way to get a `false` with that guarantee is to make the 
synchronization from outside `hasInstance()`. So, not using 
synchronization inside it does not break any contract an 
internally synchronized `hasInstance()` could promise.

>> 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.

`atomicLoad` requires a `shared` reference. Using it will force 
me to turn the singleton into a shared singleton. Maybe I should 
add a shared version(I'll return to that at the end of this 
response), but I still want to keep the __gshared version.

> 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.

First and foremost - this is library library code, so it should 
expose as much interface as possible without exposing or breaking 
the implementation.

Personally, I think `hasInstance()` would be mainly used in the 
second use case you suggested. It would be useful if you have a 
long running loop(for example - a game loop) that needs to 
interact with the singleton instance if it exists, but can't or 
shouldn't instantiate it.

As for the first use case, you are right - it is a bad design to 
busy-wait for a singleton to be instantiated somewhere else. I 
should probably add another method that waits for the instance 
using a condition.

> 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.

There is also a thread local version called 
`ThreadLocalSingleton`. If I made a shared version, would that 
solve those possible hidden race conditions? Is there a point in 
using the TLS Low Lock method for shared singletons?


More information about the Digitalmars-d mailing list