11

I know that AtomicReference has compareAndSet, but I feel like what I want to do is this

private final AtomicReference<Boolean> initialized = new AtomicReference<>( false );
...

atomicRef.compareSetAndDo( false, true, () -> {
  // stuff that only happens if false
});

this would probably work too, might be better.

atomicRef.compareAndSet( false, () -> {
  // stuff that only happens if false
  // if I die still false.

   return true;
});

I've noticed there's some new functional constructs but I'm not sure if any of them are what I'm looking for.

Can any of the new constructs do this? if so please provide an example.

update To attempt to simplify my problem, I'm trying to find a less error prone way to guard code in a "do once for object" or (really) lazy initializer fashion, and I know that some developers on my team find compareAndSet confusing.

xenoterracide
  • 16,274
  • 24
  • 118
  • 243
  • 1
    What are `false` and `true` here? Are you storing a `Boolean` in the `AtomicReference`? If so, you should be using `AtomicBoolean` instead. – Misha Oct 21 '15 at 17:02
  • @Misha updated question – xenoterracide Oct 21 '15 at 17:17
  • 1
    @xenoterracide You or I have missed something. Lazy initialisation necessarly requires lock mechanism that by definition `Atomic` API haven't. Why ? Just because system caller require system to be initialized and should be blocked as soon as initialization complete whatever it's done by the caller or another thread. May you should rely on a reactive/promise API such as `CompetableFuture` ? – LoganMzz Oct 21 '15 at 20:01
  • @LoganMzz maybe, I'm open to code examples in answer form. I really haven't looked at future's hardly at all, and examples seem less than other things on the net, and the few I found were suggesting thread pooling which would be overkill for this. – xenoterracide Oct 22 '15 at 04:09
  • I'm still confused. Do you need multithread support ? `Atomic` API (and all concurrency tools) only apply when many threads competing for the same resource. Otherwise, what other threads are supposed to do while initialization is in progress ? – LoganMzz Oct 22 '15 at 06:55
  • @LoganMzz honestly we probably don't, it's just been one solution we looked at, my question may be informing the solution too much. – xenoterracide Oct 22 '15 at 22:20
  • @xenoterracide Without any requirements, it's pretty hard to answer the question. Approximate question can only lead to approximate answer. Define requirement about multithreading and concurrency (what to do during initialization). – LoganMzz Oct 23 '15 at 07:27
  • Related, possibly a duplicate: https://stackoverflow.com/questions/20087173/how-to-do-a-lazy-create-and-set-with-atomicreference-in-a-safe-and-efficient-man – Cody Gray - on strike Jun 06 '21 at 09:20

4 Answers4

5

guard code in a "do once for object"

how exactly to implement that depends on what you want other threads attempting to execute the same thing in the meantime. if you just let them run past the CAS they may observe things in an intermediate state while the one thread that succeeded does its action.

or (really) lazy initializer fashion

that construct is not thread-safe if you're using it for lazy initializers because the "is initialized" boolean may be set to true by one thread and then execute the block while another thread observes the true-state but reads an empty result.

You can use Atomicreference::updateAndGet if multiple concurrent/repeated initialization attempts are acceptable with one object winning in the end and the others being discarded by GC. The update method should be side-effect-free.

Otherwise you should just use the double checked locking pattern with a variable reference field.

Of course you can always package any of these into a higher order function that returns a Runnable or Supplier which you then assign to a final field.

// ==  FunctionalUtils.java

/** @param mayRunMultipleTimes must be side-effect-free */
public static <T> Supplier<T> instantiateOne(Supplier<T> mayRunMultipleTimes) {
  AtomicReference<T> ref = new AtomicReference<>(null);

  return () -> {
    T val = ref.get(); // fast-path if already initialized
    if(val != null)
      return val;
    return ref.updateAndGet(v -> v == null ? mayRunMultipleTimes.get() : v)
  };

}


// == ClassWithLazyField.java

private final Supplier<Foo> lazyInstanceVal = FunctionalUtils.instantiateOne(() -> new Foo());

public Foo getFoo() {
  lazyInstanceVal.get();
}

You can easily encapsulate various custom control-flow and locking patterns this way. Here are two of my own..

the8472
  • 40,999
  • 5
  • 70
  • 122
3

compareAndSet returns true if the update was done, and false if the actual value was not equal to the expected value.

So just use

if (ref.compareAndSet(expectedValue, newValue)) { 
    ... 
}

That said, I don't really understand your examples, since you're passing true and false to a method taking object references as argument. And your second example doesn't do the same thing as the first one. If the second is what you want, I think what you're after is

ref.getAndUpdate(value -> {
    if (value.equals(expectedValue)) {
        return someNewValue(value);
    }
    else {
        return value;
    }
});
JB Nizet
  • 678,734
  • 91
  • 1,224
  • 1,255
  • I updated my question to make it more clear (I think) though not sure that it changes your answer. – xenoterracide Oct 21 '15 at 17:38
  • 1
    Given your addendum, you shouldn't use an AtomicReference, but an AtomicBoolean, and use `if (atomicBoolean.compareAndSet(false, true)) { initializeOnce() }` – JB Nizet Oct 21 '15 at 17:51
2

You’re over-complicating things. Just because there are now lambda expression, you don’t need to solve everything with lambdas:

private volatile boolean initialized;
…
if(!initialized) synchronized(this) {
    if(!initialized) {
        // stuff to be done exactly once
        initialized=true;
    }
}

The double checked locking might not have a good reputation, but for non-static properties, there are little alternatives.

If you consider multiple threads accessing it concurrently in the uninitialized state and want a guaranty that the action runs only once, and that it has completed, before dependent code is executed, an Atomic… object won’t help you.

There’s only one thread that can successfully perform compareAndSet(false,true), but since failure implies that the flag already has the new value, i.e. is initialized, all other threads will proceed as if the “stuff to be done exactly once” has been done while it might still be running. The alternative would be reading the flag first and conditionally perform the stuff and compareAndSet afterwards, but that allows multiple concurrent executions of “stuff”. This is also what happens with updateAndGet or accumulateAndGet and it’s provided function.

To guaranty exactly one execution before proceeding, threads must get blocked, if the “stuff” is currently executed. The code above does this. Note that once the “stuff” has been done, there will be no locking anymore and the performance characteristics of the volatile read are the same as for the Atomic… read.

The only solution which is simpler in programming, is to use a ConcurrentMap:

private final ConcurrentHashMap<String,Boolean> initialized=new ConcurrentHashMap<>();
…
initialized.computeIfAbsent("dummy", ignore -> {
    // stuff to do exactly once
    return true;
});

It might look a bit oversized, but it provides exactly the required performance characteristics. It will guard the initial computation using synchronized (or well, an implementation dependent exclusion mechanism) but perform a single read with volatile semantics on subsequent queries.

If you want a more lightweight solution, you may stay with the double checked locking shown at the beginning of this answer…

Holger
  • 285,553
  • 42
  • 434
  • 765
  • just a comment on the reason I'm looking at lambda's, it's because I (and others) forget to set the value at the end, been trying to find a better way to not forget to do things, the API's have made it too easy to do things wrong. whereas a lambda that requires you to return a value makes it very hard to forget to do that. – xenoterracide Oct 22 '15 at 22:15
  • Then you should separate the code. See, the first example consists of six lines only—it’s easy to maintain. Just replace the “stuff” comment with a *single method invocation* and the code keeps being that maintainable. Within the method, the code can get as complex as you want, but there you can’t forget the assignment as the method isn’t responsible for it. It’s the same principle as with a lambda, especially like with the second example, as the actual return value is irrelevant. It’s just about splitting the responsibilities, whereas it doesn’t matter whether you use methods or lambdas. – Holger Oct 23 '15 at 08:06
  • this is more of a pattern that we keep repeating across multiple objects. So saying you can't forget because it isn't responsible... well no I just implemented it again and forgot again. the thing inside the conditional is 1, or in some case 2 or 3 methods that are encapsulated (legacy in the latter). – xenoterracide Oct 23 '15 at 14:33
0

I know this is old, but I've found there is no perfect way to achieve this, more specifically this:


trying to find a less error prone way to guard code in a "do (anything) once..."


I'll add to this "while respecting a happens before behavior." which is required for instantiating singletons in your case.

IMO The best way to achieve this is by means of a synchronized function:

public<T> T transaction(Function<NonSyncObject, T> transaction) {
    synchronized (lock) {
        return transaction.apply(nonSyncObject);
    }
}

This allows to preform atomic "transactions" on the given object.

Other options are double-check spin-locks:

for (;;) {
    T t = atomicT.get();
    T newT = new T();
    if (atomicT.compareAndSet(t, newT)) return;
}

On this one new T(); will get executed repeatedly until the value is set successfully, so it is not really a "do something once".

This would only work on copy on write transactions, and could help on "instantiating objects once" (which in reality is instantiating many but at the end is referencing the same) by tweaking the code.

The final option is a worst performant version of the first one, but this one is a true happens before AND ONCE (as opposed to the double-check spin-lock):

public void doSomething(Runnable r) {
    while (!atomicBoolean.compareAndSet(false, true)) {}
    // Do some heavy stuff ONCE
    r.run();
    atomicBoolean.set(false);
}

The reason why the first one is the better option is that it is doing what this one does, but in a more optimized way.

As a side note, in my projects I've actually used the code below (similar to @the8472's answer), that at the time I thought safe, and it may be:

public T get() {
    T res = ref.get();
    if (res == null) {
        res = builder.get();
        if (ref.compareAndSet(null, res))
            return res;
        else
            return ref.get();
    } else {
        return res;
    }
}

The thing about this code is that, as the copy on write loop, this one generates multiple instances, one for each contending thread, but only one is cached, the first one, all the other constructions eventually get GC'd.

Looking at the putIfAbsent method I see the benefit is the skipping of 17 lines of code and then a synchronized body:

/** Implementation for put and putIfAbsent */
final V putVal(K key, V value, boolean onlyIfAbsent) {
    if (key == null || value == null) throw new NullPointerException();
    int hash = spread(key.hashCode());
    int binCount = 0;
    for (Node<K,V>[] tab = table;;) {
        Node<K,V> f; int n, i, fh;
        if (tab == null || (n = tab.length) == 0)
            tab = initTable();
        else if ((f = tabAt(tab, i = (n - 1) & hash)) == null) {
            if (casTabAt(tab, i, null,
                         new Node<K,V>(hash, key, value, null)))
                break;                   // no lock when adding to empty bin
        }
        else if ((fh = f.hash) == MOVED)
            tab = helpTransfer(tab, f);
        else {
            V oldVal = null;
            synchronized (f) {
                if (tabAt(tab, i) == f) {

And then the synchronized body itself is another 34 lines:

            synchronized (f) {
                if (tabAt(tab, i) == f) {
                    if (fh >= 0) {
                        binCount = 1;
                        for (Node<K,V> e = f;; ++binCount) {
                            K ek;
                            if (e.hash == hash &&
                                ((ek = e.key) == key ||
                                 (ek != null && key.equals(ek)))) {
                                oldVal = e.val;
                                if (!onlyIfAbsent)
                                    e.val = value;
                                break;
                            }
                            Node<K,V> pred = e;
                            if ((e = e.next) == null) {
                                pred.next = new Node<K,V>(hash, key,
                                                          value, null);
                                break;
                            }
                        }
                    }
                    else if (f instanceof TreeBin) {
                        Node<K,V> p;
                        binCount = 2;
                        if ((p = ((TreeBin<K,V>)f).putTreeVal(hash, key,
                                                       value)) != null) {
                            oldVal = p.val;
                            if (!onlyIfAbsent)
                                p.val = value;
                        }
                    }
                }
            }

The pro(s) of using a ConcurrentHashMap is that it will undoubtedly work.

Delark
  • 1,141
  • 2
  • 9
  • 15