Is `?.let` not thread safe?

# android

[jay shah]

Is ?.let not thread safe?

[gildor]

It’s not really about thread safety, because 2 separate operations cannot be thread safe anyway

[gildor]

Those 2 different samples are different, so you cannot compare them

[gildor]

to make ?.let work just use:
INSTANCE?.let {
return it // so, here let captures receiver object
}

[gildor]

Also, not relted to your question, but Singleton with params is terrible anti pattern IMO
It’s very unfortunate that it was promoted by many old Android API

[Emanuel Moecklin]

let creates a scope and the context object passed to the scope (that would be it) is immutable but you're accessing a property outside the scope (INSTANCE) and whatever is outside the scope might not be immutable. Change the code to

INSTANCE?.let { return it }

and the error goes away

[DALDEI]

Error (in IDE) goes away, problem does not.
by having the early return OUTSIDE of syncronize there is an order of assigment race condition. the syncronize block does not protect the
INSTANCE?.let case so it is possible for 2 instances to be created

[DALDEI]

The pattern

if( variable != null ) return variable
synchronized {
variable = Construtor()
return variable
}

is a known anti-pattern that actually does not work
An example (of many) why
http://www.cs.umd.edu/~pugh/java/memoryModel/DoubleCheckedLocking.html

[DALDEI]

Wrap the whole thing in syncronzized or use lazy

[gildor]

Wrapping whole code with synchronised will slowdown every access of the singleton

having the early return OUTSIDE of syncronize there is an order of assigment race condition

I don’t see how it a problem in this case, I mean the fact that variable is checked outside of sycnronized, it’s completely valid optimisation as soon as INSTANCE is volatile to prevent reordering.
But agree, missing null check inside of synchronized will allow creation of multiple instances

So just adding additional null check inside of synchronized block will fix the problem (so will be standard double-checked lock)

Lazy or Object would be great, but unfortunately, this is not standard singleton, this is singleton with parameter, which causes this issue. Instead of finding threading issues, I would rather restructure code and get rid of this anti-pattern completely, DI for help

UPD: BTW, article which David shared shows that it valid solution with double check and volatile field (see @Fixing Double-Checked Locking using Volatile” section)

[DALDEI]

If constructing 2 objects is not a problem (vs having 2 accessable instances) -- use of atomics may make this a lock free pattern.
Similarly, if access can be made universally via a suspend function, then use of Mutex may be more effecient