salmonalive
salmonalive

Reputation: 73

Is AtomicReference needed for visibility between threads?

I'm working with a framework that requires a callback when sending a request. Each callback has to implement this interface. The methods in the callback are invoked asynchronously.

public interface ClientCallback<RESP extends Response>
{
  public void onSuccessResponse(RESP resp);

  public void onFailureResponse(FailureResponse failure);

  public void onError(Throwable e);
}

To write integration tests with TestNG, I wanted to have a blocking callback. So I used a CountDownLatch to synchronize between threads.

Is the AtomicReference really needed here or is a raw reference okay? I know that if I use a raw reference and a raw integer (instead of CountDownLatch), the code wouldn't work because visibility is not guaranteed. But since the CountDownLatch is already synchronized, I wasn't sure whether I needed the extra synchronization from AtomicReference. Note: The Result class is immutable.

public class BlockingCallback<RESP extends Response> implements ClientCallback<RESP>
{
  private final AtomicReference<Result<RESP>> _result = new AtomicReference<Result<RESP>>();
  private final CountDownLatch _latch = new CountDownLatch(1);

  public void onSuccessResponse(RESP resp)
  {
    _result.set(new Result<RESP>(resp, null, null));
    _latch.countDown();
  }

  public void onFailureResponse(FailureResponse failure)
  {
    _result.set(new Result<RESP>(null, failure, null));
    _latch.countDown();
  }

  public void onError(Throwable e)
  {
    _result.set(new Result<RESP>(null, null, e));
    _latch.countDown();
  }

  public Result<RESP> getResult(final long timeout, final TimeUnit unit) throws InterruptedException, TimeoutException
  {
    if (!_latch.await(timeout, unit))
    {
      throw new TimeoutException();
    }
    return _result.get();
  }

Upvotes: 5

Views: 1290

Answers (4)

dimo414
dimo414

Reputation: 48864

In order for an assignment to be visible across threads some sort of memory barrier must be crossed. This can be accomplished several different ways, depending on what exactly you're trying to do.

  • You can use a volatile field. Reads and writes to volatile fields are atomic and visible across threads.
  • You can use an AtomicReference. This is effectively the same as a volatile field, but it's a little more flexible (you can reassign and pass around references to the AtomicReference) and has a few extra operations, like compareAndSet().
  • You can use a CountDownLatch or similar synchronizer class, but you need to pay close attention to the memory invariants they offer. CountDownLatch, for instance, guarantees that all threads that await() will see everything that occurs in a thread that calls countDown() up to when countDown() is called.
  • You can use synchronized blocks. These are even more flexible, but require more care - both the write and the read must be synchronized, otherwise the write may not be seen.
  • You can use a thread-safe collection, such as a ConcurrentHashMap. Overkill if all you need is a cross-thread reference, but useful for storing structured data that multiple threads need to access.

This isn't intended to be a complete list of options, but hopefully you can see there are several ways to ensure a value becomes visible to other threads, and that AtomicReference is simply one of those mechanisms.

Upvotes: 0

assylias
assylias

Reputation: 328785

A good starting point is the javadoc (emphasis mine):

Memory consistency effects: Until the count reaches zero, actions in a thread prior to calling countDown() happen-before actions following a successful return from a corresponding await() in another thread.

Now there are two options:

  1. either you never call the onXxx setter methods once the count is 0 (i.e. you only call one of the methods once) and you don't need any extra synchronization
  2. or you may call the setter methods more than once and you do need extra synchronization

If you are in scenario 2, you need to make the variable at least volatile (no need for an AtomicReference in your example).

If you are in scenario 1, you need to decide how defensive you want to be:

  • to err on the safe side you can still use volatile
  • if you are happy that the calling code won't mess up with the class, you can use a normal variable but I would at least make it clear in the javadoc of the methods that only the first call to the onXxx methods is guaranteed to be visible

Finally, in scenario 1, you may want to enforce the fact that the setters can only be called once, in which case you would probably use an AtomicReference and its compareAndSet method to make sure that the reference was null beforehand and throw an exception otherwise.

Upvotes: 1

Zbynek Vyskovsky - kvr000
Zbynek Vyskovsky - kvr000

Reputation: 18855

You don't need to use another synchronization object (AtomicRefetence) here. The point is that the variable is set before CountDownLatch is invoked in one thread and read after CountDownLatch is invoked in another thread. CountDownLatch already performs thread synchronization and invokes memory barrier so the order of writing before and reading after is guaranteed. Because of this you don't even need to use volatile for that field.

Upvotes: 2

11thdimension
11thdimension

Reputation: 10653

Short answer is you don't need AtomicReference here. You'll need volatile though.

The reason is that you're only writing to and reading from the reference (Result) and not doing any composite operations like compareAndSet().

Reads and writes are atomic for reference variables and for most primitive variables (all types except long and double).

Reference, Sun Java tutorial
https://docs.oracle.com/javase/tutorial/essential/concurrency/atomic.html

Then there is JLS (Java Language Specification)

Writes to and reads of references are always atomic, regardless of whether they are implemented as 32-bit or 64-bit values.

Java 8
http://docs.oracle.com/javase/specs/jls/se8/html/jls-17.html#jls-17.7
Java 7
http://docs.oracle.com/javase/specs/jls/se7/html/jls-17.html#jls-17.7
Java 6
http://docs.oracle.com/javase/specs/jls/se6/html/memory.html#17.7

Source : https://docs.oracle.com/javase/tutorial/essential/concurrency/atomic.html

Atomic actions cannot be interleaved, so they can be used without fear of thread interference. However, this does not eliminate all need to synchronize atomic actions, because memory consistency errors are still possible. Using volatile variables reduces the risk of memory consistency errors, because any write to a volatile variable establishes a happens-before relationship with subsequent reads of that same variable. This means that changes to a volatile variable are always visible to other threads. What's more, it also means that when a thread reads a volatile variable, it sees not just the latest change to the volatile, but also the side effects of the code that led up the change.

Since you have only single operation write/read and it's atomic, making the variable volatile will suffice.

Regarding use of CountDownLatch, it's used to wait for n operations in other threads to complete. Since you have only one operation, you can use Condition, instead of CountDownLatch.

If you're interested in usage of AtomicReference, you can check Java Concurrency in Practice (Page 326), find the book below:

https://github.com/HackathonHackers/programming-ebooks/tree/master/Java

Or the same example used by @Binita Bharti in following StackOverflow answer
When to use AtomicReference in Java?

Upvotes: 0

Related Questions