Getting a singleton calls the initializer() every time

[apatrida]

When you call to get the singleton, the putIfAbsent() calls the initializer every time a get is done. That could cause unintended consequences. especially if the initializer is expensive or has side effect.

[vinc3m1]

Good catch. Originally was using getOrPut but was concerned about thread safety. Will look into this more. Happy to accept Pull Requests if you have ideas.

[apatrida]

Not perfect but the only hole is a race condition of N threads racing to initialize at same time could call factory more than once, but all will return same instance. Without JDK 8 with internal computeIfAbsent, it is harder without more locking to be surer. I'll see what M13 implementation looks like as well.

public inline fun <K, V> ConcurrentMap<K, V>.concurrentGetOrPut(key: K, defaultValue: () -> V): V {
    var answer = this.get(key)
    if (answer == null) {
        answer = defaultValue()
        var temp = this.putIfAbsent(key, answer!!)
        if (temp != null) {
            answer = temp
        }
    }
    return answer!!
}

[apatrida]

Ah, the M13 std-lib version is mostly the same logic, just prettier:

/**
 * Concurrent getOrPut, that is safe for concurrent maps.
 *
 * Returns the value for the given [key]. If the key is not found in the map, calls the [defaultValue] function,
 * puts its result into the map under the given key and returns it.
 *
 * This method guarantees not to put the value into the map if the key is already there,
 * but the [defaultValue] function may be invoked even if the key is already in the map.
 */
public inline fun <K, V: Any> ConcurrentMap<K, V>.concurrentGetOrPut(key: K, defaultValue: () -> V): V {
    // Do not use computeIfAbsent on JVM8 as it would change locking behavior
    return this.get(key) ?:
            defaultValue().let { default -> this.putIfAbsent(key, default) ?: default }

}

[vinc3m1]

Ah nice, makes sense. Thanks!